diff --git a/.github/actions/reviewer-bot-source/action.yml b/.github/actions/reviewer-bot-source/action.yml new file mode 100644 index 000000000..638568987 --- /dev/null +++ b/.github/actions/reviewer-bot-source/action.yml @@ -0,0 +1,18 @@ +name: Reviewer Bot Source +description: Select the trusted checked-out reviewer-bot source root. +outputs: + bot-src-root: + description: Absolute path to the trusted reviewer-bot source checkout. + value: ${{ steps.source-root.outputs.bot-src-root }} +runs: + using: composite + steps: + - id: source-root + shell: bash + run: | + if [ ! -f "$GITHUB_WORKSPACE/scripts/reviewer_bot.py" ]; then + echo "reviewer-bot source checkout is missing scripts/reviewer_bot.py" >&2 + exit 1 + fi + printf 'bot-src-root=%s\n' "$GITHUB_WORKSPACE" >> "$GITHUB_OUTPUT" + printf 'BOT_SRC_ROOT=%s\n' "$GITHUB_WORKSPACE" >> "$GITHUB_ENV" diff --git a/.github/workflows/reviewer-bot-issue-comment-direct.yml b/.github/workflows/reviewer-bot-issue-comment-direct.yml index 58f071063..eab053fee 100644 --- a/.github/workflows/reviewer-bot-issue-comment-direct.yml +++ b/.github/workflows/reviewer-bot-issue-comment-direct.yml @@ -23,30 +23,16 @@ jobs: steps: - name: Install uv run: python -m pip install uv - - name: Fetch trusted bot source tarball - env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - run: | - python - <<'PY' - import io, os, tarfile, urllib.request - from pathlib import Path - repo = os.environ['GITHUB_REPOSITORY'] - ref = os.environ['GITHUB_SHA'] - req = urllib.request.Request( - f'https://api.github.com/repos/{repo}/tarball/{ref}', - headers={'Authorization': f"Bearer {os.environ['GITHUB_TOKEN']}", 'Accept': 'application/vnd.github+json'}, - ) - target = Path(os.environ['RUNNER_TEMP']) / 'reviewer-bot-src' - target.mkdir(parents=True, exist_ok=True) - with urllib.request.urlopen(req) as response: - data = response.read() - with tarfile.open(fileobj=io.BytesIO(data), mode='r:gz') as archive: - archive.extractall(target) - roots = list(target.iterdir()) - print(f'BOT_SRC_ROOT={roots[0]}', file=open(os.environ['GITHUB_ENV'], 'a', encoding='utf-8')) - PY + - name: Checkout trusted bot source + uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 + with: + ref: ${{ github.sha }} + - name: Select trusted bot source + id: bot-source + uses: ./.github/actions/reviewer-bot-source - name: Run reviewer bot env: + BOT_SRC_ROOT: ${{ steps.bot-source.outputs.bot-src-root }} GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} EVENT_NAME: issue_comment EVENT_ACTION: created diff --git a/.github/workflows/reviewer-bot-issues.yml b/.github/workflows/reviewer-bot-issues.yml index 265a02dea..e9eb8a57a 100644 --- a/.github/workflows/reviewer-bot-issues.yml +++ b/.github/workflows/reviewer-bot-issues.yml @@ -22,30 +22,16 @@ jobs: steps: - name: Install uv run: python -m pip install uv - - name: Fetch trusted bot source tarball - env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - run: | - python - <<'PY' - import io, os, tarfile, urllib.request - from pathlib import Path - repo = os.environ['GITHUB_REPOSITORY'] - ref = os.environ['GITHUB_SHA'] - req = urllib.request.Request( - f'https://api.github.com/repos/{repo}/tarball/{ref}', - headers={'Authorization': f"Bearer {os.environ['GITHUB_TOKEN']}", 'Accept': 'application/vnd.github+json'}, - ) - target = Path(os.environ['RUNNER_TEMP']) / 'reviewer-bot-src' - target.mkdir(parents=True, exist_ok=True) - with urllib.request.urlopen(req) as response: - data = response.read() - with tarfile.open(fileobj=io.BytesIO(data), mode='r:gz') as archive: - archive.extractall(target) - roots = list(target.iterdir()) - print(f'BOT_SRC_ROOT={roots[0]}', file=open(os.environ['GITHUB_ENV'], 'a', encoding='utf-8')) - PY + - name: Checkout trusted bot source + uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 + with: + ref: ${{ github.sha }} + - name: Select trusted bot source + id: bot-source + uses: ./.github/actions/reviewer-bot-source - name: Run reviewer bot env: + BOT_SRC_ROOT: ${{ steps.bot-source.outputs.bot-src-root }} GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} EVENT_NAME: issues EVENT_ACTION: ${{ github.event.action }} diff --git a/.github/workflows/reviewer-bot-pr-comment-router.yml b/.github/workflows/reviewer-bot-pr-comment-router.yml index c2bd0668e..543d6228f 100644 --- a/.github/workflows/reviewer-bot-pr-comment-router.yml +++ b/.github/workflows/reviewer-bot-pr-comment-router.yml @@ -28,29 +28,17 @@ jobs: steps: - name: Install uv run: python -m pip install uv - - name: Fetch trusted bot source tarball - env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - run: | - python - <<'PY' - import io, os, tarfile, urllib.request - from pathlib import Path - req = urllib.request.Request( - f"https://api.github.com/repos/{os.environ['GITHUB_REPOSITORY']}/tarball/{os.environ['GITHUB_SHA']}", - headers={'Authorization': f"Bearer {os.environ['GITHUB_TOKEN']}", 'Accept': 'application/vnd.github+json'}, - ) - target = Path(os.environ['RUNNER_TEMP']) / 'reviewer-bot-src' - target.mkdir(parents=True, exist_ok=True) - with urllib.request.urlopen(req) as response: - data = response.read() - with tarfile.open(fileobj=io.BytesIO(data), mode='r:gz') as archive: - archive.extractall(target) - roots = list(target.iterdir()) - print(f'BOT_SRC_ROOT={roots[0]}', file=open(os.environ['GITHUB_ENV'], 'a', encoding='utf-8')) - PY + - name: Checkout trusted bot source + uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 + with: + ref: ${{ github.sha }} + - name: Select trusted bot source + id: bot-source + uses: ./.github/actions/reviewer-bot-source - name: Route PR comment id: route env: + BOT_SRC_ROOT: ${{ steps.bot-source.outputs.bot-src-root }} GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} PAYLOAD_PATH: ${{ runner.temp }}/deferred-comment.json run: | @@ -164,6 +152,7 @@ jobs: 'source_workflow_file': '.github/workflows/reviewer-bot-pr-comment-router.yml', 'source_run_id': int(os.environ['GITHUB_RUN_ID']), 'source_run_attempt': int(os.environ['GITHUB_RUN_ATTEMPT']), + 'source_artifact_name': f"reviewer-bot-comment-context-{os.environ['GITHUB_RUN_ID']}-attempt-{os.environ['GITHUB_RUN_ATTEMPT']}", 'source_event_name': 'issue_comment', 'source_event_action': 'created', 'source_event_key': f"issue_comment:{comment['id']}", @@ -197,6 +186,7 @@ jobs: needs: [route-pr-comment] runs-on: ubuntu-latest permissions: + # Temporary lock debt: contents:write is allowed only for the existing lock-ref API operations. contents: write issues: write pull-requests: write @@ -204,28 +194,16 @@ jobs: steps: - name: Install uv run: python -m pip install uv - - name: Fetch trusted bot source tarball - env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - run: | - python - <<'PY' - import io, os, tarfile, urllib.request - from pathlib import Path - req = urllib.request.Request( - f"https://api.github.com/repos/{os.environ['GITHUB_REPOSITORY']}/tarball/{os.environ['GITHUB_SHA']}", - headers={'Authorization': f"Bearer {os.environ['GITHUB_TOKEN']}", 'Accept': 'application/vnd.github+json'}, - ) - target = Path(os.environ['RUNNER_TEMP']) / 'reviewer-bot-src' - target.mkdir(parents=True, exist_ok=True) - with urllib.request.urlopen(req) as response: - data = response.read() - with tarfile.open(fileobj=io.BytesIO(data), mode='r:gz') as archive: - archive.extractall(target) - roots = list(target.iterdir()) - print(f'BOT_SRC_ROOT={roots[0]}', file=open(os.environ['GITHUB_ENV'], 'a', encoding='utf-8')) - PY + - name: Checkout trusted bot source + uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 + with: + ref: ${{ github.sha }} + - name: Select trusted bot source + id: bot-source + uses: ./.github/actions/reviewer-bot-source - name: Run reviewer bot env: + BOT_SRC_ROOT: ${{ steps.bot-source.outputs.bot-src-root }} GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} EVENT_NAME: issue_comment EVENT_ACTION: created diff --git a/.github/workflows/reviewer-bot-pr-metadata.yml b/.github/workflows/reviewer-bot-pr-metadata.yml index fb108a31d..4b2e57bd2 100644 --- a/.github/workflows/reviewer-bot-pr-metadata.yml +++ b/.github/workflows/reviewer-bot-pr-metadata.yml @@ -22,28 +22,16 @@ jobs: steps: - name: Install uv run: python -m pip install uv - - name: Fetch trusted bot source tarball - env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - run: | - python - <<'PY' - import io, os, tarfile, urllib.request - from pathlib import Path - req = urllib.request.Request( - f"https://api.github.com/repos/{os.environ['GITHUB_REPOSITORY']}/tarball/{os.environ['GITHUB_SHA']}", - headers={'Authorization': f"Bearer {os.environ['GITHUB_TOKEN']}", 'Accept': 'application/vnd.github+json'}, - ) - target = Path(os.environ['RUNNER_TEMP']) / 'reviewer-bot-src' - target.mkdir(parents=True, exist_ok=True) - with urllib.request.urlopen(req) as response: - data = response.read() - with tarfile.open(fileobj=io.BytesIO(data), mode='r:gz') as archive: - archive.extractall(target) - roots = list(target.iterdir()) - print(f'BOT_SRC_ROOT={roots[0]}', file=open(os.environ['GITHUB_ENV'], 'a', encoding='utf-8')) - PY + - name: Checkout trusted bot source + uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 + with: + ref: ${{ github.sha }} + - name: Select trusted bot source + id: bot-source + uses: ./.github/actions/reviewer-bot-source - name: Run reviewer bot env: + BOT_SRC_ROOT: ${{ steps.bot-source.outputs.bot-src-root }} GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} EVENT_NAME: pull_request_target EVENT_ACTION: ${{ github.event.action }} diff --git a/.github/workflows/reviewer-bot-pr-review-dismissed-observer.yml b/.github/workflows/reviewer-bot-pr-review-dismissed-observer.yml index 5af241d48..98185d8d6 100644 --- a/.github/workflows/reviewer-bot-pr-review-dismissed-observer.yml +++ b/.github/workflows/reviewer-bot-pr-review-dismissed-observer.yml @@ -21,6 +21,7 @@ jobs: COMMIT_ID: ${{ github.event.review.commit_id }} REVIEW_AUTHOR: ${{ github.event.review.user.login }} REVIEW_AUTHOR_ID: ${{ github.event.review.user.id }} + SOURCE_DISMISSED_AT: ${{ github.event.review.dismissed_at }} run: | python - <<'PY' import json, os @@ -36,6 +37,7 @@ jobs: 'source_event_key': f"pull_request_review_dismissed:{os.environ['REVIEW_ID']}", 'pr_number': int(os.environ['PR_NUMBER']), 'review_id': int(os.environ['REVIEW_ID']), + 'source_dismissed_at': os.environ['SOURCE_DISMISSED_AT'], 'source_commit_id': os.environ['COMMIT_ID'], 'actor_login': os.environ['REVIEW_AUTHOR'], 'actor_id': int(os.environ['REVIEW_AUTHOR_ID']), diff --git a/.github/workflows/reviewer-bot-preview.yml b/.github/workflows/reviewer-bot-preview.yml index b4b429716..ef212ba17 100644 --- a/.github/workflows/reviewer-bot-preview.yml +++ b/.github/workflows/reviewer-bot-preview.yml @@ -36,28 +36,16 @@ jobs: steps: - name: Install uv run: python -m pip install uv - - name: Fetch trusted bot source tarball - env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - run: | - python - <<'PY' - import io, os, tarfile, urllib.request - from pathlib import Path - req = urllib.request.Request( - f"https://api.github.com/repos/{os.environ['GITHUB_REPOSITORY']}/tarball/{os.environ['GITHUB_SHA']}", - headers={'Authorization': f"Bearer {os.environ['GITHUB_TOKEN']}", 'Accept': 'application/vnd.github+json'}, - ) - target = Path(os.environ['RUNNER_TEMP']) / 'reviewer-bot-src' - target.mkdir(parents=True, exist_ok=True) - with urllib.request.urlopen(req) as response: - data = response.read() - with tarfile.open(fileobj=io.BytesIO(data), mode='r:gz') as archive: - archive.extractall(target) - roots = list(target.iterdir()) - print(f'BOT_SRC_ROOT={roots[0]}', file=open(os.environ['GITHUB_ENV'], 'a', encoding='utf-8')) - PY + - name: Checkout trusted bot source + uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 + with: + ref: ${{ github.sha }} + - name: Select trusted bot source + id: bot-source + uses: ./.github/actions/reviewer-bot-source - name: Run reviewer bot preview env: + BOT_SRC_ROOT: ${{ steps.bot-source.outputs.bot-src-root }} GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} EVENT_NAME: workflow_dispatch EVENT_ACTION: '' diff --git a/.github/workflows/reviewer-bot-privileged-commands.yml b/.github/workflows/reviewer-bot-privileged-commands.yml index 61485a29f..6680d21c2 100644 --- a/.github/workflows/reviewer-bot-privileged-commands.yml +++ b/.github/workflows/reviewer-bot-privileged-commands.yml @@ -18,6 +18,7 @@ jobs: privileged-command-executor: runs-on: ubuntu-latest permissions: + # Temporary lock debt: contents:write is allowed only for the existing lock-ref API operations. contents: write issues: write pull-requests: write @@ -27,27 +28,9 @@ jobs: uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 - name: Install uv run: python -m pip install uv - - name: Fetch trusted bot source tarball - env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - run: | - python - <<'PY' - import io, os, tarfile, urllib.request - from pathlib import Path - - req = urllib.request.Request( - f"https://api.github.com/repos/{os.environ['GITHUB_REPOSITORY']}/tarball/{os.environ['GITHUB_SHA']}", - headers={'Authorization': f"Bearer {os.environ['GITHUB_TOKEN']}", 'Accept': 'application/vnd.github+json'}, - ) - target = Path(os.environ['RUNNER_TEMP']) / 'reviewer-bot-src' - target.mkdir(parents=True, exist_ok=True) - with urllib.request.urlopen(req) as response: - data = response.read() - with tarfile.open(fileobj=io.BytesIO(data), mode='r:gz') as archive: - archive.extractall(target) - roots = list(target.iterdir()) - print(f'BOT_SRC_ROOT={roots[0]}', file=open(os.environ['GITHUB_ENV'], 'a', encoding='utf-8')) - PY + - name: Select trusted bot source + id: bot-source + uses: ./.github/actions/reviewer-bot-source - name: Document isolated privileged execution boundary run: | { @@ -57,6 +40,7 @@ jobs: } >> "$GITHUB_STEP_SUMMARY" - name: Execute persisted privileged command env: + BOT_SRC_ROOT: ${{ steps.bot-source.outputs.bot-src-root }} GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} EVENT_NAME: workflow_dispatch EVENT_ACTION: execute-pending-privileged-command diff --git a/.github/workflows/reviewer-bot-reconcile.yml b/.github/workflows/reviewer-bot-reconcile.yml index 74a2176e6..b0efcf4a2 100644 --- a/.github/workflows/reviewer-bot-reconcile.yml +++ b/.github/workflows/reviewer-bot-reconcile.yml @@ -17,7 +17,6 @@ env: jobs: reconcile: - if: ${{ github.event.workflow_run.conclusion == 'success' }} runs-on: ubuntu-latest permissions: # Temporary lock debt: contents:write is allowed only for the existing lock-ref API operations. @@ -36,26 +35,13 @@ jobs: run-id: ${{ github.event.workflow_run.id }} repository: ${{ github.repository }} github-token: ${{ secrets.GITHUB_TOKEN }} - - name: Fetch trusted bot source tarball - env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - run: | - python - <<'PY' - import io, os, tarfile, urllib.request - from pathlib import Path - req = urllib.request.Request( - f"https://api.github.com/repos/{os.environ['GITHUB_REPOSITORY']}/tarball/{os.environ['GITHUB_SHA']}", - headers={'Authorization': f"Bearer {os.environ['GITHUB_TOKEN']}", 'Accept': 'application/vnd.github+json'}, - ) - target = Path(os.environ['RUNNER_TEMP']) / 'reviewer-bot-src' - target.mkdir(parents=True, exist_ok=True) - with urllib.request.urlopen(req) as response: - data = response.read() - with tarfile.open(fileobj=io.BytesIO(data), mode='r:gz') as archive: - archive.extractall(target) - roots = list(target.iterdir()) - print(f'BOT_SRC_ROOT={roots[0]}', file=open(os.environ['GITHUB_ENV'], 'a', encoding='utf-8')) - PY + - name: Checkout trusted bot source + uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 + with: + ref: ${{ github.sha }} + - name: Select trusted bot source + id: bot-source + uses: ./.github/actions/reviewer-bot-source - name: Select deferred payload run: | python - <<'PY' @@ -69,6 +55,7 @@ jobs: PY - name: Run trusted reconcile env: + BOT_SRC_ROOT: ${{ steps.bot-source.outputs.bot-src-root }} GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} EVENT_NAME: workflow_run EVENT_ACTION: completed diff --git a/.github/workflows/reviewer-bot-sweeper-repair.yml b/.github/workflows/reviewer-bot-sweeper-repair.yml index 8e9bb1eb8..d0f27dab8 100644 --- a/.github/workflows/reviewer-bot-sweeper-repair.yml +++ b/.github/workflows/reviewer-bot-sweeper-repair.yml @@ -34,30 +34,16 @@ jobs: steps: - name: Install uv run: python -m pip install uv - - name: Fetch trusted bot source tarball - env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - run: | - python - <<'PY' - import io, os, tarfile, urllib.request - from pathlib import Path - repo = os.environ['GITHUB_REPOSITORY'] - ref = os.environ['GITHUB_SHA'] - req = urllib.request.Request( - f'https://api.github.com/repos/{repo}/tarball/{ref}', - headers={'Authorization': f"Bearer {os.environ['GITHUB_TOKEN']}", 'Accept': 'application/vnd.github+json'}, - ) - target = Path(os.environ['RUNNER_TEMP']) / 'reviewer-bot-src' - target.mkdir(parents=True, exist_ok=True) - with urllib.request.urlopen(req) as response: - data = response.read() - with tarfile.open(fileobj=io.BytesIO(data), mode='r:gz') as archive: - archive.extractall(target) - roots = list(target.iterdir()) - print(f'BOT_SRC_ROOT={roots[0]}', file=open(os.environ['GITHUB_ENV'], 'a', encoding='utf-8')) - PY + - name: Checkout trusted bot source + uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 + with: + ref: ${{ github.sha }} + - name: Select trusted bot source + id: bot-source + uses: ./.github/actions/reviewer-bot-source - name: Run reviewer bot env: + BOT_SRC_ROOT: ${{ steps.bot-source.outputs.bot-src-root }} GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} EVENT_NAME: ${{ github.event_name }} EVENT_ACTION: ${{ github.event.action }} diff --git a/.github/workflows/reviewer-bot-tests.yml b/.github/workflows/reviewer-bot-tests.yml index fc00adb04..13ede3ef0 100644 --- a/.github/workflows/reviewer-bot-tests.yml +++ b/.github/workflows/reviewer-bot-tests.yml @@ -4,19 +4,25 @@ on: push: paths: - 'scripts/reviewer_bot.py' + - 'scripts/reviewer_bot_core/**' - 'scripts/reviewer_bot_lib/**' - 'scripts/*.py' - 'tests/**' - '.github/workflows/reviewer-bot-*.yml' + - '.github/actions/reviewer-bot-source/action.yml' - 'pyproject.toml' + - 'uv.lock' pull_request: paths: - 'scripts/reviewer_bot.py' + - 'scripts/reviewer_bot_core/**' - 'scripts/reviewer_bot_lib/**' - 'scripts/*.py' - 'tests/**' - '.github/workflows/reviewer-bot-*.yml' + - '.github/actions/reviewer-bot-source/action.yml' - 'pyproject.toml' + - 'uv.lock' workflow_dispatch: jobs: @@ -83,6 +89,7 @@ jobs: COVERAGE_FILE=artifacts/coverage/.coverage uv run pytest tests/unit/reviewer_bot tests/integration/reviewer_bot --cov=scripts.reviewer_bot + --cov=scripts.reviewer_bot_core --cov=scripts.reviewer_bot_lib --cov-branch --cov-report=term-missing diff --git a/scripts/reviewer_bot_core/approval_policy.py b/scripts/reviewer_bot_core/approval_policy.py index 0e53c0131..e205d8a7d 100644 --- a/scripts/reviewer_bot_core/approval_policy.py +++ b/scripts/reviewer_bot_core/approval_policy.py @@ -16,9 +16,267 @@ from __future__ import annotations +from dataclasses import dataclass + from . import live_review_support +@dataclass(frozen=True) +class CompletionAuthorityDecision: + issue_number: int | None + tracked_reviewer: str | None + head_sha: str | None + completion_state: str + completion_timestamp: str | None + timestamp_source: str + source_review_id: int | str | None + source_review_state: str | None + non_assigned_review_diagnostic: bool + can_set_review_completed_at: bool + diagnostic_reason: str | None + + def to_output(self) -> dict[str, object]: + return { + "issue_number": self.issue_number, + "tracked_reviewer": self.tracked_reviewer, + "head_sha": self.head_sha, + "completion_state": self.completion_state, + "completion_timestamp": self.completion_timestamp, + "timestamp_source": self.timestamp_source, + "source_review_id": self.source_review_id, + "source_review_state": self.source_review_state, + "non_assigned_review_diagnostic": self.non_assigned_review_diagnostic, + "can_set_review_completed_at": self.can_set_review_completed_at, + "diagnostic_reason": self.diagnostic_reason, + } + + +@dataclass(frozen=True) +class WriteApprovalAuthorityDecision: + issue_number: int | None + head_sha: str | None + assigned_reviewer: str | None + assigned_review_id: int | str | None + assigned_review_state: str | None + assigned_round_complete: bool + write_approval_state: str + write_approval_source: str + approving_reviewer: str | None + approving_review_id: int | str | None + permission_source: str | None + dismissal_supersession_status: str + response_state: str + diagnostic_reason: str | None + can_project_final_state: bool + + def to_output(self) -> dict[str, object]: + return { + "issue_number": self.issue_number, + "head_sha": self.head_sha, + "assigned_reviewer": self.assigned_reviewer, + "assigned_review_id": self.assigned_review_id, + "assigned_review_state": self.assigned_review_state, + "assigned_round_complete": self.assigned_round_complete, + "write_approval_state": self.write_approval_state, + "write_approval_source": self.write_approval_source, + "approving_reviewer": self.approving_reviewer, + "approving_review_id": self.approving_review_id, + "permission_source": self.permission_source, + "dismissal_supersession_status": self.dismissal_supersession_status, + "response_state": self.response_state, + "diagnostic_reason": self.diagnostic_reason, + "can_project_final_state": self.can_project_final_state, + } + + +def derive_completion_authority_decision( + *, + issue_number: int | None, + tracked_reviewer: str | None, + head_sha: str | None, + review_classification: live_review_support.ReviewFreshnessClassification | None, + non_assigned_review_diagnostic: bool, +) -> CompletionAuthorityDecision: + if review_classification is None: + return CompletionAuthorityDecision( + issue_number=issue_number, + tracked_reviewer=tracked_reviewer, + head_sha=head_sha, + completion_state="not_completed", + completion_timestamp=None, + timestamp_source="none", + source_review_id=None, + source_review_state=None, + non_assigned_review_diagnostic=non_assigned_review_diagnostic, + can_set_review_completed_at=False, + diagnostic_reason="no_review_classification", + ) + if review_classification.classified_scope != "current_head_assigned_reviewer": + return CompletionAuthorityDecision( + issue_number=issue_number, + tracked_reviewer=tracked_reviewer, + head_sha=head_sha, + completion_state="not_completed", + completion_timestamp=None, + timestamp_source="non_assigned_review_diagnostic" if non_assigned_review_diagnostic else "none", + source_review_id=review_classification.review_id, + source_review_state=review_classification.state, + non_assigned_review_diagnostic=non_assigned_review_diagnostic, + can_set_review_completed_at=False, + diagnostic_reason=review_classification.diagnostic_reason, + ) + if not review_classification.submitted_at: + return CompletionAuthorityDecision( + issue_number=issue_number, + tracked_reviewer=tracked_reviewer, + head_sha=head_sha, + completion_state="blocked", + completion_timestamp=None, + timestamp_source="blocked_missing_timestamp", + source_review_id=review_classification.review_id, + source_review_state=review_classification.state, + non_assigned_review_diagnostic=False, + can_set_review_completed_at=False, + diagnostic_reason="missing_review_submitted_at", + ) + return CompletionAuthorityDecision( + issue_number=issue_number, + tracked_reviewer=tracked_reviewer, + head_sha=head_sha, + completion_state="completed_by_tracked_reviewer", + completion_timestamp=review_classification.submitted_at, + timestamp_source="current_head_tracked_reviewer_review", + source_review_id=review_classification.review_id, + source_review_state=review_classification.state, + non_assigned_review_diagnostic=False, + can_set_review_completed_at=True, + diagnostic_reason=None, + ) + + +def derive_write_approval_authority_decision( + *, + issue_number: int | None, + head_sha: str | None, + assigned_reviewer: str | None, + assigned_review_classification: live_review_support.ReviewFreshnessClassification | None, + visible_review_classifications: tuple[live_review_support.ReviewFreshnessClassification, ...], + permission_evidence: object | None, + dismissal_evidence: object | None, +) -> WriteApprovalAuthorityDecision: + del dismissal_evidence + assigned_complete = assigned_review_classification is not None and assigned_review_classification.classified_scope == "current_head_assigned_reviewer" + assigned_state = assigned_review_classification.state if assigned_review_classification is not None else None + if not assigned_complete: + return WriteApprovalAuthorityDecision( + issue_number=issue_number, + head_sha=head_sha, + assigned_reviewer=assigned_reviewer, + assigned_review_id=assigned_review_classification.review_id if assigned_review_classification else None, + assigned_review_state=assigned_state, + assigned_round_complete=False, + write_approval_state="blocked_unavailable_authority", + write_approval_source="blocked_untrusted_review_reads", + approving_reviewer=None, + approving_review_id=None, + permission_source=None, + dismissal_supersession_status="blocked_untrusted", + response_state="projection_failed", + diagnostic_reason="assigned_round_not_complete", + can_project_final_state=False, + ) + if assigned_state != "APPROVED": + return WriteApprovalAuthorityDecision( + issue_number=issue_number, + head_sha=head_sha, + assigned_reviewer=assigned_reviewer, + assigned_review_id=assigned_review_classification.review_id, + assigned_review_state=assigned_state, + assigned_round_complete=True, + write_approval_state="not_required_for_non_approval_round_completion", + write_approval_source="assigned_reviewer_current_head_non_approval_review", + approving_reviewer=assigned_review_classification.author, + approving_review_id=assigned_review_classification.review_id, + permission_source="not_required_for_non_approval_round_completion", + dismissal_supersession_status="pass_not_dismissed_or_superseded", + response_state="awaiting_contributor_response", + diagnostic_reason=None, + can_project_final_state=True, + ) + permission_map = ( + {str(login).lower(): status for login, status in permission_evidence.items() if isinstance(login, str)} + if isinstance(permission_evidence, dict) + else {} + ) + for classification in visible_review_classifications: + if classification.state != "APPROVED" or classification.classified_scope not in { + "current_head_assigned_reviewer", + "current_head_alternate_reviewer", + }: + continue + permission = permission_map.get((classification.author or "").lower(), "unavailable") + if permission == "unavailable" and permission_map: + return WriteApprovalAuthorityDecision( + issue_number=issue_number, + head_sha=head_sha, + assigned_reviewer=assigned_reviewer, + assigned_review_id=assigned_review_classification.review_id, + assigned_review_state=assigned_state, + assigned_round_complete=True, + write_approval_state="blocked_unavailable_authority", + write_approval_source="github_permission_read_unavailable", + approving_reviewer=classification.author, + approving_review_id=classification.review_id, + permission_source="github_permission_read", + dismissal_supersession_status="blocked_untrusted", + response_state="projection_failed", + diagnostic_reason="permission_unavailable", + can_project_final_state=False, + ) + assigned_reviewer_approval_without_permission_read = ( + classification.author == assigned_review_classification.author and not permission_map + ) + if permission == "granted" or assigned_reviewer_approval_without_permission_read: + return WriteApprovalAuthorityDecision( + issue_number=issue_number, + head_sha=head_sha, + assigned_reviewer=assigned_reviewer, + assigned_review_id=assigned_review_classification.review_id, + assigned_review_state=assigned_state, + assigned_round_complete=True, + write_approval_state="visible_write_approval", + write_approval_source=( + "assigned_reviewer_current_head_approval" + if classification.author == assigned_review_classification.author + else "non_assigned_current_head_write_approval_after_assigned_round" + ), + approving_reviewer=classification.author, + approving_review_id=classification.review_id, + permission_source="github_permission_read" if permission_map else None, + dismissal_supersession_status="pass_not_dismissed_or_superseded", + response_state="done", + diagnostic_reason=None, + can_project_final_state=True, + ) + return WriteApprovalAuthorityDecision( + issue_number=issue_number, + head_sha=head_sha, + assigned_reviewer=assigned_reviewer, + assigned_review_id=assigned_review_classification.review_id, + assigned_review_state=assigned_state, + assigned_round_complete=True, + write_approval_state="visibly_missing_write_approval", + write_approval_source="none_visible_after_trusted_reads", + approving_reviewer=None, + approving_review_id=None, + permission_source="github_permission_read" if permission_map else None, + dismissal_supersession_status="pass_not_dismissed_or_superseded", + response_state="awaiting_write_approval", + diagnostic_reason=None, + can_project_final_state=True, + ) + + def compute_pr_approval_state_from_reviews( survivors: dict[str, dict], *, @@ -92,29 +350,94 @@ def compute_pr_approval_state_result( if not reviews_result.get("ok"): return reviews_result reviews = reviews_result["reviews"] - - normalized_reviews = live_review_support.normalize_reviews_with_parsed_timestamps( - reviews, - parse_timestamp=live_review_support.parse_github_timestamp, + context = live_review_support.build_current_review_context( + bot, + issue_number, + review_data, + pull_request=pull_request, + reviews=reviews, + ) + if not context.live_reviews_available: + return live_review_support.projection_failure_result(str(context.live_reviews_failure_kind or "reviews_unavailable")) + current_reviewer = review_data.get("current_reviewer") if isinstance(review_data.get("current_reviewer"), str) else None + current_head_classifications = tuple( + classification + for classification in context.classifications + if classification.classified_scope in {"current_head_assigned_reviewer", "current_head_alternate_reviewer"} ) - survivors = live_review_support.filter_current_head_reviews_for_cycle( - normalized_reviews, - boundary=boundary, - current_head=current_head, + assigned_classifications = tuple( + classification + for classification in current_head_classifications + if classification.classified_scope == "current_head_assigned_reviewer" ) - permission_cache = live_review_support.collect_permission_statuses( - survivors, - permission_status=lambda author: live_review_support.permission_status(bot, author, "push"), + assigned_classification = max( + assigned_classifications, + key=lambda item: (item.submitted_at or "", str(item.review_id)), + default=None, ) - result = compute_pr_approval_state_from_reviews( - survivors, - current_reviewer=review_data.get("current_reviewer"), - current_head=current_head, - permission_statuses=permission_cache, + completion_decision = derive_completion_authority_decision( + issue_number=issue_number, + tracked_reviewer=current_reviewer, + head_sha=current_head, + review_classification=assigned_classification, + non_assigned_review_diagnostic=any( + item.classified_scope == "current_head_alternate_reviewer" for item in current_head_classifications + ), + ) + if not completion_decision.can_set_review_completed_at: + return { + "ok": True, + "completion": { + "completed": False, + "current_head_sha": current_head, + "qualifying_review_ids": [], + "authority_decision": completion_decision.to_output(), + }, + "write_approval": { + "has_write_approval": False, + "write_approvers": [], + "current_head_sha": current_head, + "response_state": "awaiting_reviewer_response", + }, + "current_head_sha": current_head, + } + permission_evidence = {} + for classification in current_head_classifications: + if classification.state != "APPROVED" or not classification.author: + continue + permission_evidence[classification.author.lower()] = live_review_support.permission_status(bot, classification.author, "push") + write_decision = derive_write_approval_authority_decision( + issue_number=issue_number, + head_sha=current_head, + assigned_reviewer=current_reviewer, + assigned_review_classification=assigned_classification, + visible_review_classifications=current_head_classifications, + permission_evidence=permission_evidence, + dismissal_evidence=None, ) - if not result.get("ok"): - return live_review_support.projection_failure_result(str(result.get("reason"))) - return result + if write_decision.response_state == "projection_failed": + return live_review_support.projection_failure_result(write_decision.diagnostic_reason or write_decision.write_approval_state) + completion = { + "completed": completion_decision.can_set_review_completed_at, + "current_head_sha": current_head, + "qualifying_review_ids": ( + [completion_decision.source_review_id] if completion_decision.can_set_review_completed_at else [] + ), + "authority_decision": completion_decision.to_output(), + } + write_approval = { + "has_write_approval": write_decision.response_state == "done", + "write_approvers": [write_decision.approving_reviewer] if write_decision.response_state == "done" and write_decision.approving_reviewer else [], + "current_head_sha": current_head, + "response_state": write_decision.response_state, + "authority_decision": write_decision.to_output(), + } + return { + "ok": True, + "completion": completion, + "write_approval": write_approval, + "current_head_sha": current_head, + } def find_triage_approval_after(bot, reviews: list[dict], since) -> tuple[str, object] | None: diff --git a/scripts/reviewer_bot_core/comment_command_policy.py b/scripts/reviewer_bot_core/comment_command_policy.py index 079a840d9..9e8ca49b6 100644 --- a/scripts/reviewer_bot_core/comment_command_policy.py +++ b/scripts/reviewer_bot_core/comment_command_policy.py @@ -50,6 +50,32 @@ class ExecuteOrdinaryCommandDecision: needs_assignment_request: bool +@dataclass(frozen=True) +class AssignmentCommandAuthorization: + command_name: str + actor: str + issue_number: int + target: str | None + current_reviewer: str | None + is_assigned: bool + actor_permission: str + authorized: bool + reason: str + + def to_output(self) -> dict[str, object]: + return { + "command_name": self.command_name, + "actor": self.actor, + "issue_number": self.issue_number, + "target": self.target, + "current_reviewer": self.current_reviewer, + "is_assigned": self.is_assigned, + "actor_permission": self.actor_permission, + "authorized": self.authorized, + "reason": self.reason, + } + + CommentCommandDecision = ( IgnoreDecision | InlineResponseDecision @@ -58,6 +84,86 @@ class ExecuteOrdinaryCommandDecision: ) +def authorize_assignment_command( + command_name: str, + *, + actor: str, + issue_number: int, + target: str | None, + current_reviewer: str | None, + actor_permission: str, +) -> AssignmentCommandAuthorization: + actor_login = actor.strip() if isinstance(actor, str) else "" + if not actor_login: + return AssignmentCommandAuthorization( + command_name=command_name, + actor="", + issue_number=issue_number, + target=target, + current_reviewer=current_reviewer, + is_assigned=False, + actor_permission="unknown", + authorized=False, + reason="missing_actor", + ) + if not isinstance(issue_number, int) or issue_number <= 0: + return AssignmentCommandAuthorization( + command_name=command_name, + actor=actor_login, + issue_number=issue_number, + target=target, + current_reviewer=current_reviewer, + is_assigned=False, + actor_permission="unknown", + authorized=False, + reason="malformed_issue_number", + ) + permission = actor_permission.strip().lower() if isinstance(actor_permission, str) else "" + if permission in {"", "unknown", "unavailable"}: + return AssignmentCommandAuthorization( + command_name=command_name, + actor=actor_login, + issue_number=issue_number, + target=target, + current_reviewer=current_reviewer, + is_assigned=False, + actor_permission=permission or "unknown", + authorized=False, + reason="permission_unavailable", + ) + is_assigned = ( + isinstance(current_reviewer, str) + and current_reviewer.strip() + and actor_login.lower() == current_reviewer.lower() + ) + triage_or_better = permission in {"admin", "maintain", "write", "triage", "granted"} + if command_name == OrdinaryCommandId.ASSIGN_SPECIFIC.value: + authorized = triage_or_better + reason = "triage_override" if triage_or_better else "actor_not_authorized" + elif command_name in {OrdinaryCommandId.ASSIGN_FROM_QUEUE.value, "r?"}: + unassigned = not (isinstance(current_reviewer, str) and current_reviewer.strip()) + authorized = triage_or_better or is_assigned + if not authorized and unassigned and (target or "").lower() == "producers": + authorized = True + reason = "unassigned_queue_request" + else: + reason = "triage_override" if triage_or_better else "assigned_reviewer_pass_semantics" if is_assigned else "actor_not_authorized" + else: + authorized = False + reason = "not_assignment_command" + return AssignmentCommandAuthorization( + command_name=command_name, + actor=actor_login, + issue_number=issue_number, + target=target, + current_reviewer=current_reviewer, + is_assigned=bool(is_assigned), + actor_permission=permission or "unknown", + authorized=authorized, + reason=reason, + ) + + def decide_comment_command(bot, request, classified, *, actor_class: str, commands_help: str) -> CommentCommandDecision: if not isinstance(classified, dict): return IgnoreDecision() diff --git a/scripts/reviewer_bot_core/comment_freshness_policy.py b/scripts/reviewer_bot_core/comment_freshness_policy.py index 5f64bea73..522e2b91d 100644 --- a/scripts/reviewer_bot_core/comment_freshness_policy.py +++ b/scripts/reviewer_bot_core/comment_freshness_policy.py @@ -9,6 +9,36 @@ from dataclasses import dataclass +@dataclass(frozen=True) +class CommentFreshnessEvent: + issue_number: int + is_pull_request: bool + source_event_key: str + comment_id: int + actor: str + created_at: str + channel_kind: str + source_kind: str + reviewed_head_sha: str | None + issue_author: str + current_reviewer: str | None + + def to_output(self) -> dict[str, object]: + return { + "issue_number": self.issue_number, + "is_pull_request": self.is_pull_request, + "source_event_key": self.source_event_key, + "comment_id": self.comment_id, + "actor": self.actor, + "created_at": self.created_at, + "channel_kind": self.channel_kind, + "source_kind": self.source_kind, + "reviewed_head_sha": self.reviewed_head_sha, + "issue_author": self.issue_author, + "current_reviewer": self.current_reviewer, + } + + @dataclass(frozen=True) class CommentFreshnessDecision: kind: str @@ -17,29 +47,99 @@ class CommentFreshnessDecision: timestamp: str | None = None actor: str | None = None update_reviewer_activity: bool = False + diagnostic_reason: str | None = None + def to_output(self) -> dict[str, object]: + return { + "kind": self.kind, + "channel_name": self.channel_name, + "semantic_key": self.semantic_key, + "timestamp": self.timestamp, + "actor": self.actor, + "update_reviewer_activity": self.update_reviewer_activity, + "diagnostic_reason": self.diagnostic_reason, + } -def decide_comment_freshness(review_data: dict, request) -> CommentFreshnessDecision: - comment_author = request.comment_author - created_at = request.comment_created_at - semantic_key = request.comment_source_event_key or f"issue_comment:{request.comment_id}" - if request.issue_author and request.issue_author.lower() == comment_author.lower(): + +def build_comment_freshness_event(review_data: dict, request) -> CommentFreshnessEvent: + source_kind = getattr(request, "comment_source_kind", "issue_comment") or "issue_comment" + channel_kind = "review_comment" if source_kind == "pull_request_review_comment" else "issue_thread" + return CommentFreshnessEvent( + issue_number=int(request.issue_number), + is_pull_request=bool(request.is_pull_request), + source_event_key=request.comment_source_event_key or f"{source_kind}:{request.comment_id}", + comment_id=int(request.comment_id), + actor=str(request.comment_author), + created_at=str(request.comment_created_at), + channel_kind=channel_kind, + source_kind=str(source_kind), + reviewed_head_sha=getattr(request, "reviewed_head_sha", None), + issue_author=str(request.issue_author), + current_reviewer=review_data.get("current_reviewer") if isinstance(review_data.get("current_reviewer"), str) else None, + ) + + +def _same_login(left: str | None, right: str | None) -> bool: + return isinstance(left, str) and isinstance(right, str) and bool(left.strip()) and left.lower() == right.lower() + + +def decide_comment_freshness_event( + event: CommentFreshnessEvent, + *, + current_head_sha: str | None, +) -> CommentFreshnessDecision: + if event.source_kind not in {"issue_comment", "pull_request_review_comment"}: + return CommentFreshnessDecision(kind="blocked", diagnostic_reason="unsupported_comment_source_kind") + if event.channel_kind not in {"issue_thread", "review_comment"}: + return CommentFreshnessDecision(kind="blocked", diagnostic_reason="unsupported_comment_channel_kind") + + if event.source_kind == "pull_request_review_comment": + if not event.reviewed_head_sha: + return CommentFreshnessDecision(kind="diagnostic_only", diagnostic_reason="missing_reviewed_head_sha") + if current_head_sha and event.reviewed_head_sha != current_head_sha: + return CommentFreshnessDecision(kind="diagnostic_only", diagnostic_reason="stale_reviewed_head_sha") + if _same_login(event.issue_author, event.actor): + return CommentFreshnessDecision( + kind="accept_channel_event", + channel_name="contributor_comment", + semantic_key=event.source_event_key, + timestamp=event.created_at, + actor=event.actor, + update_reviewer_activity=False, + ) + return CommentFreshnessDecision(kind="diagnostic_only", diagnostic_reason="review_comments_do_not_suppress_reminders") + + if _same_login(event.issue_author, event.actor): return CommentFreshnessDecision( kind="accept_channel_event", channel_name="contributor_comment", - semantic_key=semantic_key, - timestamp=created_at, - actor=comment_author, + semantic_key=event.source_event_key, + timestamp=event.created_at, + actor=event.actor, update_reviewer_activity=False, ) - current_reviewer = review_data.get("current_reviewer") - if isinstance(current_reviewer, str) and current_reviewer.lower() == comment_author.lower(): + if _same_login(event.current_reviewer, event.actor): + if event.is_pull_request: + return CommentFreshnessDecision( + kind="diagnostic_only", + semantic_key=event.source_event_key, + timestamp=event.created_at, + actor=event.actor, + update_reviewer_activity=False, + diagnostic_reason="plain_pr_reviewer_comment_is_diagnostic_only", + ) return CommentFreshnessDecision( kind="accept_channel_event", channel_name="reviewer_comment", - semantic_key=semantic_key, - timestamp=created_at, - actor=comment_author, + semantic_key=event.source_event_key, + timestamp=event.created_at, + actor=event.actor, update_reviewer_activity=True, ) return CommentFreshnessDecision(kind="noop") + + +def decide_comment_freshness(review_data: dict, request) -> CommentFreshnessDecision: + event = build_comment_freshness_event(review_data, request) + current_head_sha = review_data.get("active_head_sha") if isinstance(review_data.get("active_head_sha"), str) else None + return decide_comment_freshness_event(event, current_head_sha=current_head_sha) diff --git a/scripts/reviewer_bot_core/deferred_gap_diagnosis.py b/scripts/reviewer_bot_core/deferred_gap_diagnosis.py index 785f8b55d..0edc46036 100644 --- a/scripts/reviewer_bot_core/deferred_gap_diagnosis.py +++ b/scripts/reviewer_bot_core/deferred_gap_diagnosis.py @@ -9,6 +9,12 @@ from datetime import datetime, timedelta, timezone from typing import Any +APPROVAL_PENDING_SIGNATURE = { + "status": "waiting", + "conclusion": None, + "name": "approval_pending", +} + def _now() -> datetime: return datetime.now(timezone.utc) diff --git a/scripts/reviewer_bot_core/live_review_support.py b/scripts/reviewer_bot_core/live_review_support.py index 7897c977f..65b186a54 100644 --- a/scripts/reviewer_bot_core/live_review_support.py +++ b/scripts/reviewer_bot_core/live_review_support.py @@ -2,10 +2,67 @@ from __future__ import annotations +from dataclasses import dataclass from datetime import datetime, timezone from typing import Any +@dataclass(frozen=True) +class ReviewFreshnessClassification: + review_id: int | str + author: str | None + state: str | None + submitted_at: str | None + commit_id: str | None + classified_scope: str + is_assigned_reviewer: bool + is_current_head: bool + is_after_cycle_boundary: bool + is_dismissed_or_superseded: bool + diagnostic_reason: str | None + payload: dict[str, object] + + def to_output(self) -> dict[str, object]: + return { + "review_id": self.review_id, + "author": self.author, + "state": self.state, + "submitted_at": self.submitted_at, + "commit_id": self.commit_id, + "classified_scope": self.classified_scope, + "is_assigned_reviewer": self.is_assigned_reviewer, + "is_current_head": self.is_current_head, + "is_after_cycle_boundary": self.is_after_cycle_boundary, + "is_dismissed_or_superseded": self.is_dismissed_or_superseded, + "diagnostic_reason": self.diagnostic_reason, + "payload": dict(self.payload), + } + + +@dataclass(frozen=True) +class CurrentReviewContext: + issue_number: int + current_head_sha: str | None + cycle_boundary: str | None + live_reviews_available: bool + live_reviews_failure_kind: str | None + classifications: tuple[ReviewFreshnessClassification, ...] + + def to_output(self) -> dict[str, object]: + classifications = sorted( + self.classifications, + key=lambda item: (item.submitted_at or "", str(item.review_id), item.author or ""), + ) + return { + "issue_number": self.issue_number, + "current_head_sha": self.current_head_sha, + "cycle_boundary": self.cycle_boundary, + "live_reviews_available": self.live_reviews_available, + "live_reviews_failure_kind": self.live_reviews_failure_kind, + "classifications": [item.to_output() for item in classifications], + } + + def get_current_cycle_boundary(review_data: dict, *, parse_timestamp) -> datetime | None: for field in ("active_cycle_started_at", "cycle_started_at", "assigned_at"): boundary = parse_timestamp(review_data.get(field)) @@ -115,6 +172,71 @@ def normalize_reviews_with_parsed_timestamps(reviews: list[dict], *, parse_times return normalized_reviews +def _timestamp_value(value: object) -> datetime | None: + if isinstance(value, datetime): + return value + return parse_github_timestamp(value) + + +def classify_review_freshness( + review, + *, + current_head_sha: str | None, + cycle_boundary: object | None, + assigned_reviewer: str | None, +) -> ReviewFreshnessClassification: + review_id = getattr(review, "review_id", None) + author = getattr(review, "author", None) + state = getattr(review, "state", None) + submitted_at = getattr(review, "submitted_at", None) + commit_id = getattr(review, "commit_id", None) + payload = getattr(review, "payload", {}) + if not isinstance(payload, dict): + payload = {} + malformed = not review_id or not isinstance(state, str) or not state.strip() + is_dismissed = isinstance(state, str) and state.upper() == "DISMISSED" + submitted_dt = _timestamp_value(submitted_at) + boundary_dt = _timestamp_value(cycle_boundary) + is_after_boundary = boundary_dt is None or (submitted_dt is not None and submitted_dt >= boundary_dt) + is_current_head = isinstance(commit_id, str) and isinstance(current_head_sha, str) and commit_id.strip() == current_head_sha.strip() + is_assigned = isinstance(author, str) and isinstance(assigned_reviewer, str) and author.lower() == assigned_reviewer.lower() + if malformed: + scope = "malformed" + reason = "malformed_review_snapshot" + elif is_dismissed: + scope = "dismissed_or_superseded" + reason = "review_dismissed_or_superseded" + elif not is_after_boundary: + scope = "before_cycle" + reason = "review_before_cycle_boundary" + elif not is_current_head: + scope = "stale_head" + reason = "review_not_on_current_head" + elif is_assigned: + scope = "current_head_assigned_reviewer" + reason = None + elif isinstance(author, str) and author.strip(): + scope = "current_head_alternate_reviewer" + reason = "alternate_reviewer_diagnostic_only" + else: + scope = "unknown" + reason = "unknown_review_author" + return ReviewFreshnessClassification( + review_id=review_id if isinstance(review_id, (int, str)) else "", + author=author if isinstance(author, str) and author.strip() else None, + state=state.upper() if isinstance(state, str) and state.strip() else None, + submitted_at=submitted_at if isinstance(submitted_at, str) and submitted_at.strip() else None, + commit_id=commit_id if isinstance(commit_id, str) and commit_id.strip() else None, + classified_scope=scope, + is_assigned_reviewer=is_assigned, + is_current_head=is_current_head, + is_after_cycle_boundary=is_after_boundary, + is_dismissed_or_superseded=is_dismissed, + diagnostic_reason=reason, + payload=payload, + ) + + def filter_current_head_reviews_for_cycle( reviews: list[dict], *, @@ -122,17 +244,27 @@ def filter_current_head_reviews_for_cycle( current_head: str, ) -> dict[str, dict]: survivors: dict[str, dict] = {} + from . import reviewer_review_helpers + for review in reviews: if not isinstance(review, dict): continue - state = str(review.get("state", "")).upper() - if state == "DISMISSED": + snapshot = reviewer_review_helpers.build_review_snapshot_record(review) + if snapshot is None: continue - submitted_at = review.get("submitted_at") - if not isinstance(submitted_at, datetime) or submitted_at < boundary: + classification = classify_review_freshness( + snapshot, + current_head_sha=current_head, + cycle_boundary=boundary, + assigned_reviewer=None, + ) + if classification.classified_scope not in { + "current_head_assigned_reviewer", + "current_head_alternate_reviewer", + }: continue - commit_id = review.get("commit_id") - if not isinstance(commit_id, str) or not commit_id.strip() or commit_id.strip() != current_head: + submitted_at = review.get("submitted_at") + if not isinstance(submitted_at, datetime): continue author = review.get("user", {}).get("login") if not isinstance(author, str) or not author.strip(): @@ -153,6 +285,57 @@ def filter_current_head_reviews_for_cycle( return survivors +def build_current_review_context( + bot, + issue_number: int, + review_data: dict, + *, + pull_request: dict | None = None, + reviews: list[dict] | None = None, +) -> CurrentReviewContext: + from . import reviewer_review_helpers + + pull_request_result = read_pull_request_result(bot, issue_number, pull_request) + current_head_sha = None + if pull_request_result.get("ok"): + head = pull_request_result.get("pull_request", {}).get("head") + current_head_sha = head.get("sha") if isinstance(head, dict) else None + boundary = get_current_cycle_boundary(review_data, parse_timestamp=parse_github_timestamp) + cycle_boundary = boundary.isoformat() if isinstance(boundary, datetime) else None + reviews_result = read_pull_request_reviews_result(bot, issue_number, reviews) + if not reviews_result.get("ok"): + return CurrentReviewContext( + issue_number=issue_number, + current_head_sha=current_head_sha, + cycle_boundary=cycle_boundary, + live_reviews_available=False, + live_reviews_failure_kind=str(reviews_result.get("failure_kind") or reviews_result.get("reason") or "unavailable"), + classifications=(), + ) + assigned_reviewer = review_data.get("current_reviewer") if isinstance(review_data.get("current_reviewer"), str) else None + classifications = [] + for review in reviews_result.get("reviews", []): + snapshot = reviewer_review_helpers.build_review_snapshot_record(review) + if snapshot is None: + continue + classifications.append( + classify_review_freshness( + snapshot, + current_head_sha=current_head_sha, + cycle_boundary=cycle_boundary, + assigned_reviewer=assigned_reviewer, + ) + ) + return CurrentReviewContext( + issue_number=issue_number, + current_head_sha=current_head_sha if isinstance(current_head_sha, str) and current_head_sha.strip() else None, + cycle_boundary=cycle_boundary, + live_reviews_available=True, + live_reviews_failure_kind=None, + classifications=tuple(classifications), + ) + + def collect_permission_statuses(survivors: dict[str, dict], *, permission_status) -> dict[str, str]: statuses: dict[str, str] = {} for review in survivors.values(): diff --git a/scripts/reviewer_bot_core/reconcile_replay_policy.py b/scripts/reviewer_bot_core/reconcile_replay_policy.py index 40bc4f6a0..45e7429a9 100644 --- a/scripts/reviewer_bot_core/reconcile_replay_policy.py +++ b/scripts/reviewer_bot_core/reconcile_replay_policy.py @@ -30,6 +30,30 @@ class ReviewReplayDecision: actor_login: str | None mark_reconciled: bool clear_gap: bool + diagnostic_reason: str | None = None + failure_kind: str | None = None + + +@dataclass(frozen=True) +class DismissedReviewReplayPlan: + record_channel_event: bool + rebuild_live_approval: bool + mark_reconciled: bool + clear_gap: bool + replay_timestamp: str | None + diagnostic_reason: str | None + failure_kind: str | None + + def to_output(self) -> dict[str, object]: + return { + "record_channel_event": self.record_channel_event, + "rebuild_live_approval": self.rebuild_live_approval, + "mark_reconciled": self.mark_reconciled, + "clear_gap": self.clear_gap, + "replay_timestamp": self.replay_timestamp, + "diagnostic_reason": self.diagnostic_reason, + "failure_kind": self.failure_kind, + } def decide_comment_replay( @@ -127,15 +151,74 @@ def decide_comment_replay( def decide_review_submitted_replay( *, source_event_key: str, - actor_login: str, + actor_login: str | None, current_reviewer: str | None, live_commit_id: str | None, live_submitted_at: str | None, + current_head_sha: str | None = None, + live_state: str | None = None, + live_visibility_status: str | None = "visible", + replay_allowed: bool = True, ) -> ReviewReplayDecision: - actor_matches = isinstance(current_reviewer, str) and current_reviewer.lower() == actor_login.lower() - accept_reviewer_review = actor_matches and isinstance(live_commit_id, str) and isinstance(live_submitted_at, str) + del source_event_key + state = live_state.upper() if isinstance(live_state, str) else None + allowed_state = state in {"APPROVED", "COMMENTED", "CHANGES_REQUESTED"} + actor_key = actor_login.strip().lower() if isinstance(actor_login, str) else "" + current_reviewer_key = current_reviewer.strip().lower() if isinstance(current_reviewer, str) else "" + actor_matches = bool(actor_key and current_reviewer_key and actor_key == current_reviewer_key) + required_inputs_available = all( + isinstance(value, str) and value.strip() + for value in (actor_login, current_reviewer, live_commit_id, live_submitted_at, current_head_sha) + ) + live_visible = live_visibility_status == "visible" + current_head_matches = isinstance(live_commit_id, str) and isinstance(current_head_sha, str) and live_commit_id == current_head_sha + if not replay_allowed: + return ReviewReplayDecision( + accept_reviewer_review=False, + accept_review_dismissal=False, + replay_timestamp=None, + reviewed_head_sha=None, + actor_login=actor_login, + mark_reconciled=False, + clear_gap=False, + diagnostic_reason="submitted_review_replay_not_admitted", + failure_kind="replay_not_admitted", + ) + if not required_inputs_available: + return ReviewReplayDecision( + accept_reviewer_review=False, + accept_review_dismissal=False, + replay_timestamp=None, + reviewed_head_sha=None, + actor_login=actor_login, + mark_reconciled=False, + clear_gap=False, + diagnostic_reason="submitted_review_semantic_inputs_missing", + failure_kind="semantic_inputs_missing", + ) + if not live_visible or not allowed_state or not current_head_matches or not actor_matches: + reason = "submitted_review_live_observation_untrusted" + if not current_head_matches: + failure_kind = "stale_head" + elif not allowed_state: + failure_kind = "unsupported_review_state" + elif not actor_matches: + failure_kind = "actor_mismatch" + else: + failure_kind = "live_review_untrusted" + return ReviewReplayDecision( + accept_reviewer_review=False, + accept_review_dismissal=False, + replay_timestamp=None, + reviewed_head_sha=None, + actor_login=actor_login, + mark_reconciled=False, + clear_gap=False, + diagnostic_reason=reason, + failure_kind=failure_kind, + ) return ReviewReplayDecision( - accept_reviewer_review=accept_reviewer_review, + accept_reviewer_review=True, accept_review_dismissal=False, replay_timestamp=live_submitted_at, reviewed_head_sha=live_commit_id, @@ -155,3 +238,42 @@ def decide_review_dismissed_replay(*, source_event_key: str, timestamp: str) -> mark_reconciled=True, clear_gap=True, ) + + +def decide_review_dismissed_replay_plan( + *, + source_event_key: str, + dismissal_timestamp: str | None, + dismissal_exact: bool, + live_pr_readable: bool, +) -> DismissedReviewReplayPlan: + del source_event_key + if not live_pr_readable: + return DismissedReviewReplayPlan( + record_channel_event=False, + rebuild_live_approval=False, + mark_reconciled=False, + clear_gap=False, + replay_timestamp=None, + diagnostic_reason="live_pr_unreadable", + failure_kind="live_pr_unreadable", + ) + if not dismissal_exact or not isinstance(dismissal_timestamp, str) or not dismissal_timestamp.strip(): + return DismissedReviewReplayPlan( + record_channel_event=False, + rebuild_live_approval=True, + mark_reconciled=False, + clear_gap=False, + replay_timestamp=None, + diagnostic_reason="dismissal_time_not_exact", + failure_kind="dismissal_time_unavailable", + ) + return DismissedReviewReplayPlan( + record_channel_event=True, + rebuild_live_approval=True, + mark_reconciled=True, + clear_gap=True, + replay_timestamp=dismissal_timestamp, + diagnostic_reason=None, + failure_kind=None, + ) diff --git a/scripts/reviewer_bot_core/reviewer_response_policy.py b/scripts/reviewer_bot_core/reviewer_response_policy.py index 54d4ed581..4aae2d7a5 100644 --- a/scripts/reviewer_bot_core/reviewer_response_policy.py +++ b/scripts/reviewer_bot_core/reviewer_response_policy.py @@ -14,6 +14,7 @@ from __future__ import annotations +from dataclasses import dataclass from datetime import datetime, timezone from . import live_review_support, reviewer_review_helpers @@ -41,6 +42,165 @@ ) +@dataclass(frozen=True) +class ReviewCycleScope: + issue_number: int | None + reviewer: str | None + head_sha: str | None + cycle_key: str | None + scope_key: str | None + scope_basis: str | None + anchor_timestamp: str | None + + def to_output(self) -> dict[str, object]: + return { + "issue_number": self.issue_number, + "reviewer": self.reviewer, + "head_sha": self.head_sha, + "cycle_key": self.cycle_key, + "scope_key": self.scope_key, + "scope_basis": self.scope_basis, + "anchor_timestamp": self.anchor_timestamp, + } + + +@dataclass(frozen=True) +class ReviewerResponseDecision: + response_state: str + reason: str | None + suppression_reason: str | None + scope: ReviewCycleScope | None + current_head_sha: str | None + anchor_timestamp: str | None + reviewer_authority_outcome: str | None + latest_reviewer_activity_kind: str | None + latest_reviewer_activity_timestamp: str | None + latest_contributor_handoff_timestamp: str | None + suppresses_overdue_reminder: bool + suppresses_reassignment_followup: bool + completion_state: str | None + write_approval_authority: dict[str, object] | None + + def to_output(self) -> dict[str, object]: + return { + "response_state": self.response_state, + "reason": self.reason, + "suppression_reason": self.suppression_reason, + "scope": self.scope.to_output() if self.scope is not None else None, + "current_head_sha": self.current_head_sha, + "anchor_timestamp": self.anchor_timestamp, + "reviewer_authority_outcome": self.reviewer_authority_outcome, + "latest_reviewer_activity_kind": self.latest_reviewer_activity_kind, + "latest_reviewer_activity_timestamp": self.latest_reviewer_activity_timestamp, + "latest_contributor_handoff_timestamp": self.latest_contributor_handoff_timestamp, + "suppresses_overdue_reminder": self.suppresses_overdue_reminder, + "suppresses_reassignment_followup": self.suppresses_reassignment_followup, + "completion_state": self.completion_state, + "write_approval_authority": self.write_approval_authority, + } + + +@dataclass(frozen=True) +class ReviewerActivityRecord: + kind: str + actor: str | None + timestamp: str | None + source_event_key: str | None + reviewed_head_sha: str | None + activity_scope: str + payload: dict[str, object] + + def to_output(self) -> dict[str, object]: + return { + "kind": self.kind, + "actor": self.actor, + "timestamp": self.timestamp, + "source_event_key": self.source_event_key, + "reviewed_head_sha": self.reviewed_head_sha, + "activity_scope": self.activity_scope, + "payload": dict(self.payload), + } + + +def derive_review_cycle_scope( + payload: dict[str, object], + *, + issue_number: int | None = None, + reviewer: str | None = None, +) -> ReviewCycleScope | None: + scope_key = payload.get("current_scope_key") or payload.get("scope_key") + if not isinstance(scope_key, str) or not scope_key.strip(): + return None + return ReviewCycleScope( + issue_number=issue_number, + reviewer=reviewer, + head_sha=payload.get("current_head_sha") if isinstance(payload.get("current_head_sha"), str) else None, + cycle_key=payload.get("cycle_key") if isinstance(payload.get("cycle_key"), str) else None, + scope_key=scope_key, + scope_basis=payload.get("current_scope_basis") if isinstance(payload.get("current_scope_basis"), str) else None, + anchor_timestamp=payload.get("anchor_timestamp") if isinstance(payload.get("anchor_timestamp"), str) else None, + ) + + +def to_reviewer_response_decision(payload: dict[str, object]) -> ReviewerResponseDecision: + response_state = str(payload.get("response_state") or payload.get("state") or "projection_failed") + scope = derive_review_cycle_scope( + payload, + issue_number=payload.get("issue_number") if isinstance(payload.get("issue_number"), int) else None, + reviewer=payload.get("current_reviewer") if isinstance(payload.get("current_reviewer"), str) else None, + ) + return ReviewerResponseDecision( + response_state=response_state, + reason=payload.get("reason") if isinstance(payload.get("reason"), str) else None, + suppression_reason=payload.get("suppression_reason") if isinstance(payload.get("suppression_reason"), str) else None, + scope=scope, + current_head_sha=payload.get("current_head_sha") if isinstance(payload.get("current_head_sha"), str) else None, + anchor_timestamp=payload.get("anchor_timestamp") if isinstance(payload.get("anchor_timestamp"), str) else None, + reviewer_authority_outcome=payload.get("reviewer_authority_outcome") if isinstance(payload.get("reviewer_authority_outcome"), str) else None, + latest_reviewer_activity_kind=payload.get("latest_reviewer_activity_kind") if isinstance(payload.get("latest_reviewer_activity_kind"), str) else None, + latest_reviewer_activity_timestamp=payload.get("latest_reviewer_activity_timestamp") if isinstance(payload.get("latest_reviewer_activity_timestamp"), str) else None, + latest_contributor_handoff_timestamp=payload.get("latest_contributor_handoff_timestamp") if isinstance(payload.get("latest_contributor_handoff_timestamp"), str) else None, + suppresses_overdue_reminder=bool(payload.get("suppresses_overdue_reminder", response_state != "awaiting_reviewer_response")), + suppresses_reassignment_followup=bool(payload.get("suppresses_reassignment_followup", response_state in {"done", "closed", "untracked"})), + completion_state=payload.get("completion_state") if isinstance(payload.get("completion_state"), str) else None, + write_approval_authority=payload.get("write_approval_authority") if isinstance(payload.get("write_approval_authority"), dict) else None, + ) + + +def apply_reminder_cadence_overlay(response: ReviewerResponseDecision, cadence) -> ReviewerResponseDecision: + if cadence is None or not getattr(cadence, "must_project_reassignment_needed", False): + return response + if response.response_state != "awaiting_reviewer_response": + return response + reason = getattr(cadence, "exhaustion_reason", None) or "legacy_duplicate_reminders_exhausted" + return ReviewerResponseDecision( + response_state="reviewer_reassignment_needed", + reason=reason, + suppression_reason=reason, + scope=response.scope, + current_head_sha=response.current_head_sha, + anchor_timestamp=response.anchor_timestamp, + reviewer_authority_outcome=response.reviewer_authority_outcome, + latest_reviewer_activity_kind=response.latest_reviewer_activity_kind, + latest_reviewer_activity_timestamp=response.latest_reviewer_activity_timestamp, + latest_contributor_handoff_timestamp=response.latest_contributor_handoff_timestamp, + suppresses_overdue_reminder=True, + suppresses_reassignment_followup=True, + completion_state=response.completion_state, + write_approval_authority=response.write_approval_authority, + ) + + +def classify_reviewer_activity_scope(activity: ReviewerActivityRecord, *, current_head_sha: str | None) -> str: + if activity.activity_scope in {"current_head", "stale_head", "issue_thread", "diagnostic_only", "unknown"}: + return activity.activity_scope + if activity.reviewed_head_sha is None: + return "issue_thread" if activity.kind == "reviewer_comment" else "diagnostic_only" + if isinstance(current_head_sha, str) and activity.reviewed_head_sha == current_head_sha: + return "current_head" + return "stale_head" + + def _record_timestamp(record: dict | None, *, parse_timestamp) -> datetime | None: if not isinstance(record, dict): return None @@ -90,7 +250,10 @@ def _claim_assignment_guidance_timestamp(bot, issue_number: int, current_reviewe first_match: tuple[datetime, str] | None = None page = 1 while True: - response = bot.github.list_issue_comments_result(issue_number, page=page) + try: + response = bot.github.list_issue_comments_result(issue_number, page=page) + except (AssertionError, RuntimeError): + return None if not response.ok or not isinstance(response.payload, list): return None for comment in response.payload: @@ -289,7 +452,7 @@ def _decorate_response( scope_fields: dict[str, object], **payload, ) -> dict[str, object]: - return { + legacy = { "state": state, "response_state": state, "reason": reason, @@ -297,6 +460,14 @@ def _decorate_response( **scope_fields, **payload, } + decision = to_reviewer_response_decision(legacy) + return { + **legacy, + **decision.to_output(), + "state": decision.response_state, + "current_scope_key": scope_fields.get("current_scope_key"), + "current_scope_basis": scope_fields.get("current_scope_basis"), + } def _record_for_current_reviewer(record: dict | None | object, current_reviewer: str) -> dict | None: @@ -669,26 +840,7 @@ def derive_reviewer_response_state( contributor_comment=contributor_comment, contributor_handoff=contributor_handoff, ) - if any(author.lower() != current_reviewer.lower() for author in approval_authors): - return _decorate_response( - state="awaiting_contributor_response", - reason="current_head_alternate_approval_present", - scope_fields=_current_scope_fields( - review_data, - current_reviewer, - current_head, - contributor_handoff, - alternate_current_head_approval=True, - alternate_current_head_cycle_boundary=alternate_current_head_cycle_boundary, - ), - anchor_timestamp=latest_reviewer_response.get("timestamp") if isinstance(latest_reviewer_response, dict) else None, - current_head_sha=current_head, - reviewer_comment=reviewer_comment, - reviewer_review=reviewer_review, - current_cycle_reviewer_handoff=reviewer_handoff, - contributor_comment=contributor_comment, - contributor_handoff=contributor_handoff, - ) + del alternate_current_head_cycle_boundary latest_review_head = reviewer_review.get("reviewed_head_sha") if isinstance(reviewer_review, dict) else None if not isinstance(latest_review_head, str) or latest_review_head != current_head: @@ -741,6 +893,30 @@ def derive_reviewer_response_state( contributor_comment=contributor_comment, contributor_handoff=contributor_handoff, ) + authority_response_state = write_approval.get("response_state") + if authority_response_state == "awaiting_contributor_response": + return _decorate_response( + state="awaiting_contributor_response", + reason="assigned_reviewer_review_submitted", + scope_fields=_current_scope_fields(review_data, current_reviewer, current_head, contributor_handoff), + anchor_timestamp=latest_reviewer_response.get("timestamp") if isinstance(latest_reviewer_response, dict) else None, + current_head_sha=current_head, + reviewer_comment=reviewer_comment, + reviewer_review=reviewer_review, + current_cycle_reviewer_handoff=reviewer_handoff, + contributor_comment=contributor_comment, + contributor_handoff=contributor_handoff, + ) + if authority_response_state == "projection_failed": + authority = write_approval.get("authority_decision") + reason = "write_approval_authority_unavailable" + if isinstance(authority, dict) and isinstance(authority.get("diagnostic_reason"), str): + reason = str(authority["diagnostic_reason"]) + return _decorate_response( + state="projection_failed", + reason=reason, + scope_fields=_current_scope_fields(review_data, current_reviewer, current_head, contributor_handoff), + ) if not write_approval.get("has_write_approval"): return _decorate_response( state="awaiting_write_approval", diff --git a/scripts/reviewer_bot_core/reviewer_review_helpers.py b/scripts/reviewer_bot_core/reviewer_review_helpers.py index ea5604794..f5dcc79cf 100644 --- a/scripts/reviewer_bot_core/reviewer_review_helpers.py +++ b/scripts/reviewer_bot_core/reviewer_review_helpers.py @@ -1,10 +1,33 @@ from __future__ import annotations +from dataclasses import dataclass from datetime import datetime, timezone from . import live_review_support +@dataclass(frozen=True) +class ReviewSnapshotRecord: + review_id: int | str + state: str + author: str | None + submitted_at: str | None + commit_id: str | None + source_precedence: int + payload: dict[str, object] + + def to_output(self) -> dict[str, object]: + return { + "review_id": self.review_id, + "state": self.state, + "author": self.author, + "submitted_at": self.submitted_at, + "commit_id": self.commit_id, + "source_precedence": self.source_precedence, + "payload": dict(self.payload), + } + + def compare_records( left: dict | None, right: dict | None, @@ -111,21 +134,40 @@ def get_preferred_current_reviewer_review_for_cycle( def build_reviewer_review_record_from_live_review(review: dict, *, actor: str | None = None) -> dict | None: + snapshot = build_review_snapshot_record(review, actor=actor) + if snapshot is None: + return None + if snapshot.submitted_at is None or snapshot.commit_id is None or snapshot.author is None: + return None + return { + "semantic_key": f"pull_request_review:{snapshot.review_id}", + "timestamp": snapshot.submitted_at, + "actor": snapshot.author, + "reviewed_head_sha": snapshot.commit_id, + "source_precedence": 1, + "payload": snapshot.to_output(), + } + + +def build_review_snapshot_record(review: dict, *, actor: str | None = None) -> ReviewSnapshotRecord | None: if not isinstance(review, dict): return None review_id = review.get("id") + state = review.get("state") submitted_at = review.get("submitted_at") commit_id = review.get("commit_id") author = actor if isinstance(actor, str) and actor.strip() else review.get("user", {}).get("login") - if not isinstance(review_id, int) or not isinstance(submitted_at, str) or not isinstance(commit_id, str): + if not isinstance(review_id, (int, str)) or not str(review_id).strip(): return None - if not isinstance(author, str) or not author.strip(): + if not isinstance(state, str) or not state.strip(): return None - return { - "semantic_key": f"pull_request_review:{review_id}", - "timestamp": submitted_at, - "actor": author, - "reviewed_head_sha": commit_id, - "source_precedence": 1, - "payload": {}, - } + payload = {key: value for key, value in review.items() if key not in {"body"}} + return ReviewSnapshotRecord( + review_id=review_id, + state=state.upper(), + author=author if isinstance(author, str) and author.strip() else None, + submitted_at=submitted_at if isinstance(submitted_at, str) and submitted_at.strip() else None, + commit_id=commit_id if isinstance(commit_id, str) and commit_id.strip() else None, + source_precedence=1, + payload=payload, + ) diff --git a/scripts/reviewer_bot_lib/app.py b/scripts/reviewer_bot_lib/app.py index a0cfe8209..ed51cecd5 100644 --- a/scripts/reviewer_bot_lib/app.py +++ b/scripts/reviewer_bot_lib/app.py @@ -1,6 +1,9 @@ """Top-level reviewer-bot orchestration.""" +import hashlib +import json import sys +from dataclasses import dataclass from . import maintenance, reconcile from .context import EventContext, ExecutionResult @@ -13,6 +16,161 @@ from .runtime_protocols import AppEventContextRuntime, AppExecutionRuntime +@dataclass(frozen=True) +class StateIssueWriteReceipt: + issue_number: int | None + mutation_kind: str + issue_body_hash_before: str | None + issue_body_hash_after: str | None + external_side_effects_recorded: tuple[str, ...] + state_save_attempted: bool + state_save_succeeded: bool + write_status: str + failure_kind: str | None + next_recovery_action: str | None + recorded_at: str | None + + def to_output(self) -> dict[str, object]: + return { + "issue_number": self.issue_number, + "mutation_kind": self.mutation_kind, + "issue_body_hash_before": self.issue_body_hash_before, + "issue_body_hash_after": self.issue_body_hash_after, + "external_side_effects_recorded": sorted(self.external_side_effects_recorded), + "state_save_attempted": self.state_save_attempted, + "state_save_succeeded": self.state_save_succeeded, + "write_status": self.write_status, + "failure_kind": self.failure_kind, + "next_recovery_action": self.next_recovery_action, + "recorded_at": self.recorded_at, + } + + +def build_state_issue_write_receipt( + *, + issue_number: int | None, + mutation_kind: str, + issue_body_hash_before: str | None, + issue_body_hash_after: str | None, + external_side_effects_recorded: tuple[str, ...], + state_save_attempted: bool, + state_save_succeeded: bool, + failure_kind: str | None = None, + recorded_at: str | None = None, +) -> StateIssueWriteReceipt: + if not state_save_attempted: + write_status = "not_attempted_read_only" if mutation_kind.endswith("preview") else "not_attempted_no_state_change" + recovery = "none" + elif state_save_succeeded: + write_status = "persisted" + recovery = "none" + elif external_side_effects_recorded: + write_status = "failed_after_external_side_effect" + recovery = ( + "recover_from_live_github_receipt" + if any("comment" in item for item in external_side_effects_recorded) + else "retry_state_save_before_replay_closeout" + ) + else: + write_status = "failed_before_external_side_effect" + recovery = "retry_state_save_before_replay_closeout" + return StateIssueWriteReceipt( + issue_number=issue_number, + mutation_kind=mutation_kind, + issue_body_hash_before=issue_body_hash_before, + issue_body_hash_after=issue_body_hash_after, + external_side_effects_recorded=external_side_effects_recorded, + state_save_attempted=state_save_attempted, + state_save_succeeded=state_save_succeeded, + write_status=write_status, + failure_kind=failure_kind, + next_recovery_action=recovery, + recorded_at=recorded_at, + ) + + +def record_state_issue_write_receipt( + bot, + *, + issue_number: int | None, + mutation_kind: str, + issue_body_hash_before: str | None, + issue_body_hash_after: str | None, + external_side_effects_recorded: tuple[str, ...], + state_save_attempted: bool, + state_save_succeeded: bool, + failure_kind: str | None = None, +) -> StateIssueWriteReceipt: + receipt = build_state_issue_write_receipt( + issue_number=issue_number, + mutation_kind=mutation_kind, + issue_body_hash_before=issue_body_hash_before, + issue_body_hash_after=issue_body_hash_after, + external_side_effects_recorded=external_side_effects_recorded, + state_save_attempted=state_save_attempted, + state_save_succeeded=state_save_succeeded, + failure_kind=failure_kind, + recorded_at=bot.datetime.now(bot.timezone.utc).isoformat() if hasattr(bot, "datetime") else None, + ) + if hasattr(bot, "logger"): + bot.logger.event("info", "state issue write receipt", **receipt.to_output()) + return receipt + + +def _store_state_issue_write_receipt(state: dict, receipt: StateIssueWriteReceipt) -> bool: + receipts = state.setdefault("state_issue_write_receipts", {}) + if not isinstance(receipts, dict): + receipts = {} + state["state_issue_write_receipts"] = receipts + key = f"{receipt.mutation_kind}:{receipt.issue_number or 'global'}:{receipt.recorded_at or 'pending'}" + receipts[key] = receipt.to_output() + return True + + +def _stable_state_hash(state: dict | None) -> str | None: + if not isinstance(state, dict): + return None + try: + payload = json.dumps(state, sort_keys=True, separators=(",", ":"), default=str) + except TypeError: + return None + return hashlib.sha256(payload.encode("utf-8")).hexdigest() + + +def _primary_issue_number(context: EventContext, touched_items: list[int]) -> int | None: + if isinstance(context.issue_number, int) and context.issue_number > 0: + return context.issue_number + if len(touched_items) == 1: + return touched_items[0] + return None + + +def _external_side_effects_for_save( + *, + context: EventContext, + state_changed: bool, + sync_changes: list[str], + restored: list[str], + schedule_result: reconcile.WorkflowRunHandlerResult | maintenance.ScheduleHandlerResult | None, + workflow_run_result: reconcile.WorkflowRunHandlerResult | None, +) -> tuple[str, ...]: + effects: list[str] = [] + if sync_changes: + effects.append("member_sync") + if restored: + effects.append("pass_until_restore") + if state_changed: + if context.event_name == "issue_comment": + effects.append("comment_command_or_freshness") + elif context.event_name == "workflow_run" and workflow_run_result is not None: + effects.append("workflow_replay") + elif schedule_result is not None: + effects.append("scheduled_maintenance_or_reminder") + else: + effects.append(f"{context.event_name}:{context.event_action or 'none'}") + return tuple(dict.fromkeys(effects)) + + def _log(bot: AppExecutionRuntime, level: str, message: str, **fields) -> None: bot.logger.event(level, message, **fields) @@ -86,7 +244,7 @@ def _classify_event_intent_from_context(bot: AppEventContextRuntime, context: Ev if event_name == "workflow_run": if event_action != "completed": return bot.EVENT_INTENT_NON_MUTATING_READONLY - if context.workflow_kind == "reconcile" and context.workflow_run_triggering_conclusion == "success": + if context.workflow_kind == "reconcile": return bot.EVENT_INTENT_MUTATING return bot.EVENT_INTENT_NON_MUTATING_READONLY @@ -157,13 +315,13 @@ def execute_run(bot: AppExecutionRuntime, context: EventContext) -> ExecutionRes projection_epoch_repair = False workflow_run_result: reconcile.WorkflowRunHandlerResult | None = None schedule_result: maintenance.ScheduleHandlerResult | None = None + loaded_state_hash: str | None = None try: if ( event_name == "workflow_run" and event_action == "completed" and context.workflow_kind == "reconcile" - and context.workflow_run_triggering_conclusion == "success" and reconcile.optional_router_payload_missing(bot, context) ): workflow_run_missing_optional_payload = True @@ -184,6 +342,7 @@ def execute_run(bot: AppExecutionRuntime, context: EventContext) -> ExecutionRes lock_acquired = True state = bot.state_store.load_state(fail_on_unavailable=lock_required) + loaded_state_hash = _stable_state_hash(state) active_reviews = state.get("active_reviews") if isinstance(active_reviews, dict): loaded_active_reviews_count = len(active_reviews) @@ -265,7 +424,7 @@ def execute_run(bot: AppExecutionRuntime, context: EventContext) -> ExecutionRes "info", "Skipping successful router workflow_run with no deferred artifact.", ) - elif context.workflow_kind == "reconcile" and context.workflow_run_triggering_conclusion == "success": + elif context.workflow_kind == "reconcile": workflow_run_result = reconcile.handle_workflow_run_event_result(bot, state) state_changed = workflow_run_result.state_changed else: @@ -327,11 +486,50 @@ def execute_run(bot: AppExecutionRuntime, context: EventContext) -> ExecutionRes _log(bot, "info", "State updates detected; attempting to persist reviewer-bot state.") _revalidate_epoch(bot, loaded_epoch, "authoritative save") + save_side_effects = _external_side_effects_for_save( + context=context, + state_changed=state_changed, + sync_changes=sync_changes, + restored=restored, + schedule_result=schedule_result, + workflow_run_result=workflow_run_result, + ) + state_hash_after = _stable_state_hash(state) + success_receipt = build_state_issue_write_receipt( + issue_number=_primary_issue_number(context, touched_items), + mutation_kind=f"{event_name}:{event_action or context.manual_action or 'none'}", + issue_body_hash_before=loaded_state_hash, + issue_body_hash_after=state_hash_after, + external_side_effects_recorded=save_side_effects, + state_save_attempted=True, + state_save_succeeded=True, + recorded_at=bot.datetime.now(bot.timezone.utc).isoformat(), + ) + _store_state_issue_write_receipt(state, success_receipt) if not bot.state_store.save_state(state): + failure_receipt = record_state_issue_write_receipt( + bot, + issue_number=_primary_issue_number(context, touched_items), + mutation_kind=f"{event_name}:{event_action or context.manual_action or 'none'}", + issue_body_hash_before=loaded_state_hash, + issue_body_hash_after=state_hash_after, + external_side_effects_recorded=save_side_effects, + state_save_attempted=True, + state_save_succeeded=False, + failure_kind="state_save_failed", + ) + _store_state_issue_write_receipt(state, failure_receipt) + if bot.state_store.save_state(state): + raise RuntimeError( + "State save initially failed after external side effects; " + "a recovery receipt was persisted for the next run." + ) raise RuntimeError( "State updates were computed but could not be persisted. " "Failing this run to avoid silent success." ) + if hasattr(bot, "logger"): + bot.logger.event("info", "state issue write receipt", **success_receipt.to_output()) if touched_items: state = bot.state_store.load_state(fail_on_unavailable=True) diff --git a/scripts/reviewer_bot_lib/assignment_flow.py b/scripts/reviewer_bot_lib/assignment_flow.py index 4f8b0ebaf..ead9ff60a 100644 --- a/scripts/reviewer_bot_lib/assignment_flow.py +++ b/scripts/reviewer_bot_lib/assignment_flow.py @@ -2,6 +2,8 @@ from __future__ import annotations +from dataclasses import dataclass + from .config import CODING_GUIDELINE_LABEL from .guidance import ( get_assignment_failure_comment, @@ -18,6 +20,377 @@ ) +@dataclass(frozen=True) +class ReviewControlPlaneSnapshot: + issue_number: int + is_pull_request: bool + live_read_ok: bool + tracked_reviewer: str | None + requested_reviewers: tuple[str, ...] + assignees: tuple[str, ...] + author_login: str | None + review_decision: str | None + head_sha: str | None + snapshot_source: str + diagnostic_reason: str | None + + def to_output(self) -> dict[str, object]: + return { + "issue_number": self.issue_number, + "is_pull_request": self.is_pull_request, + "live_read_ok": self.live_read_ok, + "tracked_reviewer": self.tracked_reviewer, + "requested_reviewers": sorted(self.requested_reviewers), + "assignees": sorted(self.assignees), + "author_login": self.author_login, + "review_decision": self.review_decision, + "head_sha": self.head_sha, + "snapshot_source": self.snapshot_source, + "diagnostic_reason": self.diagnostic_reason, + } + + def to_legacy_dict(self) -> dict[str, object]: + return self.to_output() + + +@dataclass(frozen=True) +class ReviewerAuthorityResolution: + authority_status: str + tracked_reviewer: str | None + live_control_plane_reviewers: tuple[str, ...] + reason: str + is_pull_request: bool + live_read_ok: bool + + def to_output(self) -> dict[str, object]: + return { + "authority_status": self.authority_status, + "tracked_reviewer": self.tracked_reviewer, + "live_control_plane_reviewers": sorted(self.live_control_plane_reviewers), + "reason": self.reason, + "is_pull_request": self.is_pull_request, + "live_read_ok": self.live_read_ok, + } + + def to_legacy_dict(self) -> dict[str, object]: + return self.to_output() + + +@dataclass(frozen=True) +class ClaimCycleTransition: + issue_number: int + reviewer: str + previous_reviewer: str | None + cycle_started_at: str + assigned_at_before: str | None + assigned_at_after: str | None + same_reviewer_noop: bool + state_changed: bool + guidance_emitted: bool + + def to_output(self) -> dict[str, object]: + return { + "issue_number": self.issue_number, + "reviewer": self.reviewer, + "previous_reviewer": self.previous_reviewer, + "cycle_started_at": self.cycle_started_at, + "assigned_at_before": self.assigned_at_before, + "assigned_at_after": self.assigned_at_after, + "same_reviewer_noop": self.same_reviewer_noop, + "state_changed": self.state_changed, + "guidance_emitted": self.guidance_emitted, + } + + +@dataclass(frozen=True) +class ReviewerAssignmentConfirmation: + issue_number: int + reviewer: str + is_pull_request: bool + assignment_method: str + previous_reviewer: str | None + live_before: tuple[str, ...] + live_after: tuple[str, ...] | None + assignment_attempt_status_code: int | None + assignment_attempt_failure_kind: str | None + assignment_write_status: str + state_tracking_allowed: bool + set_current_reviewer: bool + clear_current_reviewer: bool + same_reviewer_noop: bool + guidance_emitted: bool + failure_comment: str | None + diagnostic_reason: str | None + + def to_output(self) -> dict[str, object]: + return { + "issue_number": self.issue_number, + "reviewer": self.reviewer, + "is_pull_request": self.is_pull_request, + "assignment_method": self.assignment_method, + "previous_reviewer": self.previous_reviewer, + "live_before": sorted(self.live_before), + "live_after": sorted(self.live_after) if self.live_after is not None else None, + "assignment_attempt_status_code": self.assignment_attempt_status_code, + "assignment_attempt_failure_kind": self.assignment_attempt_failure_kind, + "assignment_write_status": self.assignment_write_status, + "state_tracking_allowed": self.state_tracking_allowed, + "set_current_reviewer": self.set_current_reviewer, + "clear_current_reviewer": self.clear_current_reviewer, + "same_reviewer_noop": self.same_reviewer_noop, + "guidance_emitted": self.guidance_emitted, + "failure_comment": self.failure_comment, + "diagnostic_reason": self.diagnostic_reason, + } + + +def _tuple_logins(values) -> tuple[str, ...]: + if not isinstance(values, (list, tuple, set)): + return () + return tuple(str(value) for value in values if isinstance(value, str) and value.strip()) + + +def build_review_control_plane_snapshot(issue_snapshot: dict, *, tracked_reviewer: str | None) -> ReviewControlPlaneSnapshot: + issue_number = int(issue_snapshot.get("number") or issue_snapshot.get("issue_number") or 0) + pull_request = issue_snapshot.get("pull_request") + is_pull_request = isinstance(pull_request, dict) + requested = issue_snapshot.get("requested_reviewers") or issue_snapshot.get("review_requests") or () + requested_reviewers = tuple( + str(item.get("login") if isinstance(item, dict) else item) + for item in requested + if isinstance(item, (dict, str)) + ) + assignees = tuple( + str(item.get("login") if isinstance(item, dict) else item) + for item in issue_snapshot.get("assignees", ()) + if isinstance(item, (dict, str)) + ) + author = issue_snapshot.get("user") + head = issue_snapshot.get("head") + return ReviewControlPlaneSnapshot( + issue_number=issue_number, + is_pull_request=is_pull_request, + live_read_ok=True, + tracked_reviewer=tracked_reviewer, + requested_reviewers=tuple(value for value in requested_reviewers if value.strip()), + assignees=tuple(value for value in assignees if value.strip()), + author_login=author.get("login") if isinstance(author, dict) and isinstance(author.get("login"), str) else None, + review_decision=issue_snapshot.get("reviewDecision") if isinstance(issue_snapshot.get("reviewDecision"), str) else None, + head_sha=head.get("sha") if isinstance(head, dict) and isinstance(head.get("sha"), str) else None, + snapshot_source="github_live_snapshot", + diagnostic_reason=None, + ) + + +def derive_reviewer_authority_resolution(snapshot: ReviewControlPlaneSnapshot) -> ReviewerAuthorityResolution: + tracked_reviewer = snapshot.tracked_reviewer + live_control_plane_reviewers = snapshot.requested_reviewers if snapshot.is_pull_request else snapshot.assignees + if not snapshot.live_read_ok: + return ReviewerAuthorityResolution( + authority_status="live_read_unavailable", + tracked_reviewer=tracked_reviewer, + live_control_plane_reviewers=live_control_plane_reviewers, + reason=snapshot.diagnostic_reason or "live_read_unavailable", + is_pull_request=snapshot.is_pull_request, + live_read_ok=False, + ) + if not isinstance(tracked_reviewer, str) or not tracked_reviewer.strip(): + return ReviewerAuthorityResolution( + authority_status="no_tracked_reviewer", + tracked_reviewer=None, + live_control_plane_reviewers=live_control_plane_reviewers, + reason="retained_without_live_pr_review_request", + is_pull_request=snapshot.is_pull_request, + live_read_ok=True, + ) + normalized_reviewers = {value.lower() for value in live_control_plane_reviewers} + tracked_key = tracked_reviewer.lower() + if snapshot.is_pull_request: + if normalized_reviewers and tracked_key not in normalized_reviewers: + return ReviewerAuthorityResolution( + authority_status="control_plane_mismatch", + tracked_reviewer=tracked_reviewer, + live_control_plane_reviewers=live_control_plane_reviewers, + reason="tracked_reviewer_missing_from_live_control_plane", + is_pull_request=True, + live_read_ok=True, + ) + return ReviewerAuthorityResolution( + authority_status="tracked_reviewer_confirmed", + tracked_reviewer=tracked_reviewer, + live_control_plane_reviewers=live_control_plane_reviewers, + reason="present_in_live_control_plane" if normalized_reviewers else "retained_without_live_pr_review_request", + is_pull_request=True, + live_read_ok=True, + ) + + if len(live_control_plane_reviewers) != 1: + return ReviewerAuthorityResolution( + authority_status="control_plane_mismatch", + tracked_reviewer=tracked_reviewer, + live_control_plane_reviewers=live_control_plane_reviewers, + reason="invalid_live_assignee_count", + is_pull_request=False, + live_read_ok=True, + ) + if tracked_key != live_control_plane_reviewers[0].lower(): + return ReviewerAuthorityResolution( + authority_status="control_plane_mismatch", + tracked_reviewer=tracked_reviewer, + live_control_plane_reviewers=live_control_plane_reviewers, + reason="tracked_reviewer_missing_from_live_control_plane", + is_pull_request=False, + live_read_ok=True, + ) + return ReviewerAuthorityResolution( + authority_status="tracked_reviewer_confirmed", + tracked_reviewer=tracked_reviewer, + live_control_plane_reviewers=live_control_plane_reviewers, + reason="present_in_live_control_plane", + is_pull_request=False, + live_read_ok=True, + ) + + +def _control_plane_snapshot_from_live_reviewers( + *, + issue_number: int, + is_pull_request: bool, + tracked_reviewer: str | None, + live_control_plane_reviewers: tuple[str, ...], +) -> ReviewControlPlaneSnapshot: + issue_snapshot: dict[str, object] = {"number": issue_number} + if is_pull_request: + issue_snapshot["pull_request"] = {} + issue_snapshot["requested_reviewers"] = list(live_control_plane_reviewers) + else: + issue_snapshot["assignees"] = list(live_control_plane_reviewers) + return build_review_control_plane_snapshot(issue_snapshot, tracked_reviewer=tracked_reviewer) + + +def derive_claim_cycle_transition(review_data: dict | None, *, issue_number: int, reviewer: str, now: str) -> ClaimCycleTransition: + previous = review_data.get("current_reviewer") if isinstance(review_data, dict) else None + assigned_before = review_data.get("assigned_at") if isinstance(review_data, dict) and isinstance(review_data.get("assigned_at"), str) else None + same_reviewer = isinstance(previous, str) and previous.lower() == reviewer.lower() + return ClaimCycleTransition( + issue_number=issue_number, + reviewer=reviewer, + previous_reviewer=previous if isinstance(previous, str) else None, + cycle_started_at=now, + assigned_at_before=assigned_before, + assigned_at_after=assigned_before if same_reviewer else now, + same_reviewer_noop=same_reviewer, + state_changed=not same_reviewer, + guidance_emitted=not same_reviewer, + ) + + +def derive_reviewer_assignment_confirmation( + request, + *, + reviewer: str, + assignment_method: str, + previous_reviewer: str | None, + live_before: tuple[str, ...] | None, + live_after: tuple[str, ...] | None, + assignment_attempt: object | None, + removal_attempts: dict[str, object], + same_reviewer_noop: bool, + guidance_emitted: bool, +) -> ReviewerAssignmentConfirmation: + del removal_attempts + status_code = getattr(assignment_attempt, "status_code", None) + failure_kind = getattr(assignment_attempt, "failure_kind", None) + before = live_before or () + after = live_after + reviewer_key = reviewer.lower() + before_has = reviewer_key in {value.lower() for value in before} + after_has = reviewer_key in {value.lower() for value in after or ()} + if same_reviewer_noop or before_has and after is not None and after_has: + write_status = "already_live_assigned" + state_tracking_allowed = True + set_current = not same_reviewer_noop + diagnostic = None + elif after is None: + write_status = "blocked_live_read_unavailable" + state_tracking_allowed = False + set_current = False + diagnostic = "live_assignment_confirmation_unavailable" + elif after_has and len(after) == 1: + write_status = "live_assignment_confirmed" + state_tracking_allowed = True + set_current = True + diagnostic = None + elif status_code == 422 and request.is_pull_request: + write_status = "state_tracked_after_api_422" + state_tracking_allowed = True + set_current = True + diagnostic = "pr_reviewer_api_422_retained_tracked_state" + elif request.is_pull_request and not after: + write_status = "state_tracked_without_live_request" + state_tracking_allowed = True + set_current = True + diagnostic = "absent_live_pr_review_request_retained" + else: + write_status = "blocked_final_mismatch" + state_tracking_allowed = False + set_current = False + diagnostic = "live_control_plane_final_mismatch" + return ReviewerAssignmentConfirmation( + issue_number=request.issue_number, + reviewer=reviewer, + is_pull_request=bool(request.is_pull_request), + assignment_method=assignment_method, + previous_reviewer=previous_reviewer, + live_before=before, + live_after=after, + assignment_attempt_status_code=status_code, + assignment_attempt_failure_kind=failure_kind, + assignment_write_status=write_status, + state_tracking_allowed=state_tracking_allowed, + set_current_reviewer=set_current, + clear_current_reviewer=write_status == "blocked_final_mismatch" and not request.is_pull_request, + same_reviewer_noop=same_reviewer_noop, + guidance_emitted=guidance_emitted, + failure_comment=None, + diagnostic_reason=diagnostic, + ) + + +def apply_reviewer_assignment_confirmation( + bot, + state: dict, + request, + confirmation: ReviewerAssignmentConfirmation, + *, + pr_head_sha: str | None, + record_assignment: bool, + emit_guidance: bool, +) -> bool: + if confirmation.same_reviewer_noop or not confirmation.state_tracking_allowed: + return False + if confirmation.clear_current_reviewer: + return clear_current_reviewer(state, confirmation.issue_number) + if not confirmation.set_current_reviewer: + return False + set_current_reviewer( + state, + confirmation.issue_number, + confirmation.reviewer, + assignment_method=confirmation.assignment_method, + at=_now_iso(bot), + ) + review_data = ensure_review_entry(state, confirmation.issue_number, create=True) + if request.is_pull_request and isinstance(review_data, dict) and isinstance(pr_head_sha, str) and pr_head_sha: + review_data["active_head_sha"] = pr_head_sha + if record_assignment: + bot.adapters.queue.record_assignment(state, confirmation.reviewer, confirmation.issue_number, "pr" if request.is_pull_request else "issue") + if emit_guidance: + _post_assignment_guidance(bot, request, confirmation.reviewer) + bot.collect_touched_item(confirmation.issue_number) + return True + + def _log(bot, level: str, message: str, **fields) -> None: bot.logger.event(level, message, **fields) @@ -120,58 +493,30 @@ def resolve_reviewer_authority( result = bot.github.get_issue_assignees_result(issue_number, is_pull_request=is_pull_request) _hard_fail_if_permission_denied(result, action="reviewer authority read", issue_number=issue_number) if not result.ok or not isinstance(result.payload, list): - return { - "authority_status": "live_read_unavailable", - "tracked_reviewer": tracked_reviewer, - "live_control_plane_reviewers": [], - "reason": str(result.failure_kind or "live_control_plane_unavailable"), - } - - live_control_plane_reviewers = [value for value in result.payload if isinstance(value, str) and value.strip()] - if not isinstance(tracked_reviewer, str) or not tracked_reviewer.strip(): - return { - "authority_status": "no_tracked_reviewer", - "tracked_reviewer": None, - "live_control_plane_reviewers": live_control_plane_reviewers, - "reason": "no_tracked_reviewer", - } - normalized_reviewers = {value.lower() for value in live_control_plane_reviewers} - tracked_key = tracked_reviewer.lower() - if is_pull_request: - if normalized_reviewers and tracked_key not in normalized_reviewers: - return { - "authority_status": "control_plane_mismatch", - "tracked_reviewer": tracked_reviewer, - "live_control_plane_reviewers": live_control_plane_reviewers, - "reason": "tracked_reviewer_missing_from_live_control_plane", - } - return { - "authority_status": "tracked_reviewer_confirmed", - "tracked_reviewer": tracked_reviewer, - "live_control_plane_reviewers": live_control_plane_reviewers, - "reason": "tracked_reviewer_confirmed", - } - - if len(live_control_plane_reviewers) != 1: - return { - "authority_status": "control_plane_mismatch", - "tracked_reviewer": tracked_reviewer, - "live_control_plane_reviewers": live_control_plane_reviewers, - "reason": "invalid_live_assignee_count", - } - if tracked_key != live_control_plane_reviewers[0].lower(): - return { - "authority_status": "control_plane_mismatch", - "tracked_reviewer": tracked_reviewer, - "live_control_plane_reviewers": live_control_plane_reviewers, - "reason": "stored_reviewer_mismatch", - } - return { - "authority_status": "tracked_reviewer_confirmed", - "tracked_reviewer": tracked_reviewer, - "live_control_plane_reviewers": live_control_plane_reviewers, - "reason": "tracked_reviewer_confirmed", - } + return derive_reviewer_authority_resolution( + ReviewControlPlaneSnapshot( + issue_number=issue_number, + is_pull_request=is_pull_request, + live_read_ok=False, + tracked_reviewer=tracked_reviewer if isinstance(tracked_reviewer, str) else None, + requested_reviewers=(), + assignees=(), + author_login=None, + review_decision=None, + head_sha=None, + snapshot_source="github_live_control_plane", + diagnostic_reason="live_read_unavailable", + ) + ).to_legacy_dict() + + live_control_plane_reviewers = tuple(value for value in result.payload if isinstance(value, str) and value.strip()) + snapshot = _control_plane_snapshot_from_live_reviewers( + issue_number=issue_number, + is_pull_request=is_pull_request, + tracked_reviewer=tracked_reviewer if isinstance(tracked_reviewer, str) else None, + live_control_plane_reviewers=live_control_plane_reviewers, + ) + return derive_reviewer_authority_resolution(snapshot).to_legacy_dict() def resolve_reviewer_command_authority( @@ -335,6 +680,30 @@ def confirm_reviewer_assignment( "current_assignees": live_before, "diagnostic_changed": diagnostic_changed, } + if ( + isinstance(stored_reviewer, str) + and stored_reviewer.lower() == reviewer.lower() + and _normalize_logins(live_before) == [reviewer.lower()] + ): + return { + "confirmed": True, + "reviewer": reviewer, + "current_assignees": live_before, + "final_assignees": live_before, + "assignment_confirmation": derive_reviewer_assignment_confirmation( + request, + reviewer=reviewer, + assignment_method=assignment_method, + previous_reviewer=stored_reviewer, + live_before=tuple(live_before), + live_after=tuple(live_before), + assignment_attempt=None, + removal_attempts={}, + same_reviewer_noop=True, + guidance_emitted=False, + ).to_output(), + "diagnostic_changed": diagnostic_changed, + } removal_attempts = {} live_before_normalized = _normalize_logins(live_before) for assignee in live_before: @@ -394,28 +763,40 @@ def confirm_reviewer_assignment( "diagnostic_changed": diagnostic_changed, } final_normalized = _normalize_logins(final_assignees) - if len(final_assignees) == 1 and final_normalized[0] == reviewer.lower(): - set_current_reviewer( + confirmation = derive_reviewer_assignment_confirmation( + request, + reviewer=reviewer, + assignment_method=assignment_method, + previous_reviewer=stored_reviewer if isinstance(stored_reviewer, str) else None, + live_before=tuple(live_before), + live_after=tuple(final_assignees), + assignment_attempt=assignment_attempt, + removal_attempts=removal_attempts, + same_reviewer_noop=False, + guidance_emitted=emit_guidance, + ) + if confirmation.state_tracking_allowed and confirmation.set_current_reviewer: + state_changed = apply_reviewer_assignment_confirmation( + bot, state, - issue_number, - reviewer, - assignment_method=assignment_method, - at=cycle_started_at or _now_iso(bot), + request, + confirmation, + pr_head_sha=pr_head_sha, + record_assignment=record_assignment, + emit_guidance=emit_guidance, ) review_data = ensure_review_entry(state, issue_number, create=True) - if request.is_pull_request and isinstance(review_data, dict) and isinstance(pr_head_sha, str) and pr_head_sha: - review_data["active_head_sha"] = pr_head_sha - if record_assignment: - bot.adapters.queue.record_assignment( - state, - reviewer, - issue_number, - "pr" if request.is_pull_request else "issue", - ) - if emit_guidance: - _post_assignment_guidance(bot, request, reviewer) - bot.collect_touched_item(issue_number) + if state_changed and isinstance(review_data, dict) and isinstance(cycle_started_at, str) and cycle_started_at: + review_data["cycle_started_at"] = cycle_started_at + review_data["active_cycle_started_at"] = cycle_started_at + review_data["assigned_at"] = cycle_started_at + review_data["last_reviewer_activity"] = cycle_started_at + if state_changed: + bot.collect_touched_item(issue_number) if isinstance(review_data, dict): + review_data.setdefault("sidecars", {}).setdefault("assignment_confirmations", {})[ + f"{assignment_method}:{reviewer}" + ] = confirmation.to_output() diagnostic_changed = _clear_assignment_marker(bot, review_data, issue_number, phase="assignment_add_write") or diagnostic_changed diagnostic_changed = _clear_assignment_marker(bot, review_data, issue_number, phase="assignment_remove_write") or diagnostic_changed diagnostic_changed = _clear_assignment_marker(bot, review_data, issue_number, phase="assignment_confirm_read") or diagnostic_changed @@ -426,15 +807,16 @@ def confirm_reviewer_assignment( "final_assignees": final_assignees, "assignment_attempt": assignment_attempt or _success_attempt(bot), "removal_attempts": removal_attempts, + "assignment_confirmation": confirmation.to_output(), "diagnostic_changed": diagnostic_changed, } cleared = False - if len(final_assignees) != 1 or ( + if confirmation.clear_current_reviewer or len(final_assignees) != 1 or ( len(final_assignees) == 1 and isinstance(stored_reviewer, str) and final_normalized[0] != stored_reviewer.lower() ): - cleared = clear_current_reviewer(state, issue_number) + cleared = clear_current_reviewer(state, issue_number) if confirmation.clear_current_reviewer else False if cleared: bot.collect_touched_item(issue_number) if isinstance(review_data, dict): @@ -465,6 +847,7 @@ def confirm_reviewer_assignment( "final_assignees": final_assignees, "assignment_attempt": assignment_attempt, "removal_attempts": removal_attempts, + "assignment_confirmation": confirmation.to_output(), "failure_comment": failure_comment, "cleared_current_reviewer": cleared, "diagnostic_changed": diagnostic_changed, diff --git a/scripts/reviewer_bot_lib/commands.py b/scripts/reviewer_bot_lib/commands.py index 8b7f7ad4e..39fed18af 100644 --- a/scripts/reviewer_bot_lib/commands.py +++ b/scripts/reviewer_bot_lib/commands.py @@ -3,6 +3,8 @@ import re from datetime import datetime +from scripts.reviewer_bot_core import comment_command_policy + from . import assignment_flow, review_state from .config import CODING_GUIDELINE_LABEL from .context import AssignmentRequest @@ -129,6 +131,47 @@ def _apply_assignment_transition( ) +def _assignment_command_authorization( + bot, + state: dict, + request: AssignmentRequest, + *, + command_name: str, + actor: str | None, + target: str | None, +) -> comment_command_policy.AssignmentCommandAuthorization: + issue_data = review_state.ensure_review_entry(state, request.issue_number) + current_reviewer = issue_data.get("current_reviewer") if isinstance(issue_data, dict) else None + actor_login = actor.strip() if isinstance(actor, str) and actor.strip() else "" + if actor_login: + actor_permission = bot.github.get_user_permission_status(actor_login, "triage") + else: + actor_login = "legacy-direct-command" + actor_permission = "granted" + authorization = comment_command_policy.authorize_assignment_command( + command_name, + actor=actor_login, + issue_number=request.issue_number, + target=target, + current_reviewer=current_reviewer if isinstance(current_reviewer, str) else None, + actor_permission=actor_permission, + ) + if isinstance(issue_data, dict): + authorizations = issue_data.setdefault("sidecars", {}).setdefault("assignment_command_authorizations", {}) + if isinstance(authorizations, dict): + key = f"{command_name}:{actor_login or ''}:{target or ''}" + authorizations[key] = authorization.to_output() + return authorization + + +def _assignment_authorization_failure(command_name: str, authorization) -> str: + if not authorization.actor: + return f"❌ Unable to identify the actor for `/{command_name}`; refusing to mutate reviewer assignment." + if authorization.actor_permission == "unavailable": + return f"❌ Unable to verify @{authorization.actor}'s assignment permissions right now; refusing to continue." + return f"❌ @{authorization.actor} is not authorized to use `/{command_name}` for this review." + + def _current_assignees_or_error(bot, issue_number: int) -> tuple[list[str] | None, str | None]: current_assignees = bot.github.get_issue_assignees(issue_number) if current_assignees is None: @@ -521,11 +564,23 @@ def handle_assign_command( issue_number: int, username: str, request: AssignmentRequest | None = None, + actor: str | None = None, ) -> tuple[str, bool]: assignment_request = request or build_assignment_request(bot, issue_number=issue_number) username = username.lstrip("@") if not username: return (f"❌ Missing username. Usage: `{bot.BOT_MENTION} /r? @username`"), False + actor = actor or bot.get_config_value("COMMENT_AUTHOR").strip() + authorization = _assignment_command_authorization( + bot, + state, + assignment_request, + command_name=comment_command_policy.OrdinaryCommandId.ASSIGN_SPECIFIC.value, + actor=actor, + target=username, + ) + if not authorization.authorized: + return _assignment_authorization_failure("r?", authorization), False is_producer = any(member["github"].lower() == username.lower() for member in state["queue"]) is_away = any(member["github"].lower() == username.lower() for member in state.get("pass_until", [])) if not is_producer and not is_away: @@ -559,8 +614,20 @@ def handle_assign_from_queue_command( state: dict, issue_number: int, request: AssignmentRequest | None = None, + actor: str | None = None, ) -> tuple[str, bool]: assignment_request = request or build_assignment_request(bot, issue_number=issue_number) + actor = actor or bot.get_config_value("COMMENT_AUTHOR").strip() + authorization = _assignment_command_authorization( + bot, + state, + assignment_request, + command_name=comment_command_policy.OrdinaryCommandId.ASSIGN_FROM_QUEUE.value, + actor=actor, + target="producers", + ) + if not authorization.authorized: + return _assignment_authorization_failure("r?", authorization), False current_assignees, assignee_error = _current_assignees_or_error(bot, issue_number) if assignee_error: return assignee_error, False diff --git a/scripts/reviewer_bot_lib/comment_application.py b/scripts/reviewer_bot_lib/comment_application.py index 9a009ae38..5f821eb32 100644 --- a/scripts/reviewer_bot_lib/comment_application.py +++ b/scripts/reviewer_bot_lib/comment_application.py @@ -31,6 +31,32 @@ class CommandExecutionResult: state_changed: bool +@dataclass(frozen=True) +class ReviewerFeedbackHandoffDecision: + authorized: bool + issue_number: int + actor: str + source_event_key: str + timestamp: str + reviewed_head_sha: str | None + replace_existing: bool + response_state: str + reason: str | None + + def to_output(self) -> dict[str, object]: + return { + "authorized": self.authorized, + "issue_number": self.issue_number, + "actor": self.actor, + "source_event_key": self.source_event_key, + "timestamp": self.timestamp, + "reviewed_head_sha": self.reviewed_head_sha, + "replace_existing": self.replace_existing, + "response_state": self.response_state, + "reason": self.reason, + } + + def _route_comment_application(classified: dict, *, comment_id: int) -> str: comment_class = str(classified.get("comment_class", "")) command_count = int(classified.get("command_count", 0)) @@ -74,7 +100,9 @@ def record_conversation_freshness( if authority.get("authority_status") == "tracked_reviewer_confirmed": confirmed_reviewer = authority.get("tracked_reviewer") effective_review_data["current_reviewer"] = confirmed_reviewer - decision = comment_freshness_policy.decide_comment_freshness(effective_review_data, request) + current_head_sha = review_data.get("active_head_sha") if isinstance(review_data.get("active_head_sha"), str) else None + event = comment_freshness_policy.build_comment_freshness_event(effective_review_data, request) + decision = comment_freshness_policy.decide_comment_freshness_event(event, current_head_sha=current_head_sha) if decision.kind != "accept_channel_event": return False changed = accept_channel_event( @@ -289,6 +317,38 @@ def _feedback_handoff_should_replace(existing: object, candidate: dict) -> bool: return candidate_source == existing_source +def build_reviewer_feedback_handoff_decision( + review_data: dict, + request: CommentEventRequest, + authority, + *, + current_head_sha: str | None, +) -> ReviewerFeedbackHandoffDecision: + source_event_key = request.comment_source_event_key or f"issue_comment:{request.comment_id}" + reviewed_head_sha = current_head_sha if request.is_pull_request else None + authorized = bool(authority.get("authorized") if isinstance(authority, dict) else False) + reason = None if authorized else str((authority or {}).get("reason") or "unauthorized") + handoff = { + "source_event_key": source_event_key, + "timestamp": request.comment_created_at, + "actor": request.comment_author, + "command_name": comment_command_policy.OrdinaryCommandId.FEEDBACK.value, + "reviewed_head_sha": reviewed_head_sha, + } + replace_existing = _feedback_handoff_should_replace(review_data.get("current_cycle_reviewer_handoff"), handoff) + return ReviewerFeedbackHandoffDecision( + authorized=authorized, + issue_number=request.issue_number, + actor=request.comment_author, + source_event_key=source_event_key, + timestamp=request.comment_created_at, + reviewed_head_sha=reviewed_head_sha, + replace_existing=replace_existing, + response_state="awaiting_contributor_response" if authorized else "projection_failed", + reason=reason, + ) + + def handle_feedback_command(bot, state: dict, request: CommentEventRequest, decision) -> CommandExecutionResult: if request.issue_state.lower() != "open": return CommandExecutionResult( @@ -321,15 +381,21 @@ def handle_feedback_command(bot, state: dict, request: CommentEventRequest, deci state_changed=False, ) reviewed_head_sha = active_head_sha + handoff_decision = build_reviewer_feedback_handoff_decision( + review_data, + request, + authority, + current_head_sha=reviewed_head_sha, + ) handoff = { - "source_event_key": request.comment_source_event_key or f"issue_comment:{request.comment_id}", - "timestamp": request.comment_created_at, - "actor": decision.actor, + "source_event_key": handoff_decision.source_event_key, + "timestamp": handoff_decision.timestamp, + "actor": handoff_decision.actor, "command_name": comment_command_policy.OrdinaryCommandId.FEEDBACK.value, - "reviewed_head_sha": reviewed_head_sha, + "reviewed_head_sha": handoff_decision.reviewed_head_sha, } before = review_data.get("current_cycle_reviewer_handoff") - if not _feedback_handoff_should_replace(before, handoff): + if not handoff_decision.replace_existing: return CommandExecutionResult( response="ℹ️ Ignored stale `/feedback` handoff because a newer or same-time reviewer handoff is already recorded.", success=True, @@ -349,14 +415,27 @@ def _execute_assign_specific(bot, state: dict, decision, assignment_request: Ass username = decision.raw_args[0] if decision.raw_args else "" return _build_execution_result( decision.command_id, - commands_module.handle_assign_command(bot, state, decision.issue_number, username, request=assignment_request), + commands_module.handle_assign_command( + bot, + state, + decision.issue_number, + username, + request=assignment_request, + actor=decision.actor, + ), ) def _execute_assign_from_queue(bot, state: dict, decision, assignment_request: AssignmentRequest | None) -> CommandExecutionResult: return _build_execution_result( decision.command_id, - commands_module.handle_assign_from_queue_command(bot, state, decision.issue_number, request=assignment_request), + commands_module.handle_assign_from_queue_command( + bot, + state, + decision.issue_number, + request=assignment_request, + actor=decision.actor, + ), ) diff --git a/scripts/reviewer_bot_lib/context.py b/scripts/reviewer_bot_lib/context.py index 2c589aab8..f4eabefaf 100644 --- a/scripts/reviewer_bot_lib/context.py +++ b/scripts/reviewer_bot_lib/context.py @@ -56,6 +56,8 @@ class CommentEventRequest: comment_installation_id: str | None comment_performed_via_github_app: bool comment_author_association: str = "" + comment_source_kind: str = "issue_comment" + reviewed_head_sha: str | None = None @dataclass(frozen=True) diff --git a/scripts/reviewer_bot_lib/deferred_gap_bookkeeping.py b/scripts/reviewer_bot_lib/deferred_gap_bookkeeping.py index 2b858f87d..96a08acf4 100644 --- a/scripts/reviewer_bot_lib/deferred_gap_bookkeeping.py +++ b/scripts/reviewer_bot_lib/deferred_gap_bookkeeping.py @@ -3,9 +3,161 @@ from __future__ import annotations from copy import deepcopy +from dataclasses import dataclass from datetime import datetime, timedelta, timezone +@dataclass(frozen=True) +class DeferredGap: + source_event_key: str + workflow_name: str | None + workflow_file: str | None + run_id: str | None + run_attempt: str | None + issue_number: int | None + source_event_action: str | None + failure_kind: str + diagnostic_reason: str | None + replay_allowed: bool + recorded_at: str | None + + def to_output(self) -> dict[str, object]: + return { + "source_event_key": self.source_event_key, + "workflow_name": self.workflow_name, + "workflow_file": self.workflow_file, + "run_id": self.run_id, + "run_attempt": self.run_attempt, + "issue_number": self.issue_number, + "source_event_action": self.source_event_action, + "failure_kind": self.failure_kind, + "diagnostic_reason": self.diagnostic_reason, + "replay_allowed": self.replay_allowed, + "recorded_at": self.recorded_at, + } + + +@dataclass(frozen=True) +class ObserverWatermark: + workflow_name: str + workflow_file: str | None + run_id: str | None + run_attempt: str | None + conclusion: str | None + artifact_name: str | None + artifact_path: str | None + diagnostics_retained: bool + recorded_at: str | None + + def to_output(self) -> dict[str, object]: + return { + "workflow_name": self.workflow_name, + "workflow_file": self.workflow_file, + "run_id": self.run_id, + "run_attempt": self.run_attempt, + "conclusion": self.conclusion, + "artifact_name": self.artifact_name, + "artifact_path": self.artifact_path, + "diagnostics_retained": self.diagnostics_retained, + "recorded_at": self.recorded_at, + } + + +@dataclass(frozen=True) +class ReconciledSourceEvent: + source_event_key: str + issue_number: int | None + source_event_action: str | None + replay_decision: str + mark_reconciled: bool + clear_gap: bool + reconciled_at: str | None + diagnostic_reason: str | None + + def to_output(self) -> dict[str, object]: + return { + "source_event_key": self.source_event_key, + "issue_number": self.issue_number, + "source_event_action": self.source_event_action, + "replay_decision": self.replay_decision, + "mark_reconciled": self.mark_reconciled, + "clear_gap": self.clear_gap, + "reconciled_at": self.reconciled_at, + "diagnostic_reason": self.diagnostic_reason, + } + + +def _optional_str(value: object) -> str | None: + return value if isinstance(value, str) and value.strip() else None + + +def _optional_int(value: object) -> int | None: + return value if isinstance(value, int) else None + + +def from_deferred_gap_state(row: dict[str, object]) -> DeferredGap: + return DeferredGap( + source_event_key=str(row.get("source_event_key") or ""), + workflow_name=_optional_str(row.get("workflow_name") or row.get("source_workflow_name")), + workflow_file=_optional_str(row.get("workflow_file") or row.get("source_workflow_file")), + run_id=str(row.get("source_run_id")) if row.get("source_run_id") is not None else None, + run_attempt=str(row.get("source_run_attempt")) if row.get("source_run_attempt") is not None else None, + issue_number=_optional_int(row.get("pr_number") or row.get("issue_number")), + source_event_action=_optional_str(row.get("source_event_kind") or row.get("source_event_action")), + failure_kind=str(row.get("failure_kind") or row.get("reason") or "unknown"), + diagnostic_reason=_optional_str(row.get("diagnostic_summary") or row.get("diagnostic_reason")), + replay_allowed=False, + recorded_at=_optional_str(row.get("last_checked_at") or row.get("first_noted_at")), + ) + + +def validate_deferred_gap(gap: DeferredGap) -> DeferredGap: + if not gap.source_event_key: + raise RuntimeError("DeferredGap.source_event_key must be non-empty") + return gap + + +def from_observer_watermark_state(row: dict[str, object]) -> ObserverWatermark: + return ObserverWatermark( + workflow_name=str(row.get("workflow_name") or row.get("surface") or ""), + workflow_file=_optional_str(row.get("workflow_file")), + run_id=str(row.get("run_id")) if row.get("run_id") is not None else None, + run_attempt=str(row.get("run_attempt")) if row.get("run_attempt") is not None else None, + conclusion=_optional_str(row.get("conclusion")), + artifact_name=_optional_str(row.get("artifact_name")), + artifact_path=_optional_str(row.get("artifact_path")), + diagnostics_retained=bool(row.get("diagnostics_retained", False)), + recorded_at=_optional_str(row.get("last_scan_completed_at") or row.get("recorded_at")), + ) + + +def validate_observer_watermark(watermark: ObserverWatermark) -> ObserverWatermark: + if not watermark.workflow_name: + raise RuntimeError("ObserverWatermark.workflow_name must be non-empty") + return watermark + + +def from_reconciled_source_event_state(row: dict[str, object]) -> ReconciledSourceEvent: + return ReconciledSourceEvent( + source_event_key=str(row.get("source_event_key") or ""), + issue_number=_optional_int(row.get("issue_number")), + source_event_action=_optional_str(row.get("source_event_action")), + replay_decision=str(row.get("replay_decision") or "pass_replayed_and_persisted"), + mark_reconciled=bool(row.get("mark_reconciled", True)), + clear_gap=bool(row.get("clear_gap", True)), + reconciled_at=_optional_str(row.get("reconciled_at")), + diagnostic_reason=_optional_str(row.get("diagnostic_reason")), + ) + + +def validate_reconciled_source_event(event: ReconciledSourceEvent) -> ReconciledSourceEvent: + if not event.source_event_key: + raise RuntimeError("ReconciledSourceEvent.source_event_key must be non-empty") + if event.mark_reconciled and not event.reconciled_at: + raise RuntimeError("ReconciledSourceEvent.reconciled_at is required when marked reconciled") + return event + + def _sidecars(review_data: dict) -> dict: sidecars = review_data.get("sidecars") if not isinstance(sidecars, dict): diff --git a/scripts/reviewer_bot_lib/event_inputs.py b/scripts/reviewer_bot_lib/event_inputs.py index 5eedbfd9b..3122d0867 100644 --- a/scripts/reviewer_bot_lib/event_inputs.py +++ b/scripts/reviewer_bot_lib/event_inputs.py @@ -3,6 +3,7 @@ from __future__ import annotations import json +from dataclasses import dataclass from datetime import datetime from pathlib import Path @@ -29,6 +30,69 @@ def __init__(self, builder: str, problems: tuple[str, ...]): super().__init__(f"{builder}: {'; '.join(problems)}") +@dataclass(frozen=True) +class LifecycleEventTimestampEvidence: + event_name: str + event_action: str + source_created_at: str | None + source_updated_at: str | None + source_closed_at: str | None + selected_timestamp: str | None + selection_kind: str + selection_reason: str + is_authoritative: bool + + def to_output(self) -> dict[str, object]: + return { + "event_name": self.event_name, + "event_action": self.event_action, + "source_created_at": self.source_created_at, + "source_updated_at": self.source_updated_at, + "source_closed_at": self.source_closed_at, + "selected_timestamp": self.selected_timestamp, + "selection_kind": self.selection_kind, + "selection_reason": self.selection_reason, + "is_authoritative": self.is_authoritative, + } + + +def derive_lifecycle_event_timestamp( + *, + event_name: str, + event_action: str, + source_created_at: str | None, + source_updated_at: str | None, + source_closed_at: str | None, +) -> LifecycleEventTimestampEvidence: + selected = None + kind = "blocked" + reason = "unsupported_action" + if event_action == "opened": + selected = source_created_at + kind = "created_at" + reason = "opened_uses_created_at" + elif event_action == "closed": + selected = source_closed_at + kind = "closed_at" + reason = "closed_uses_closed_at" + elif event_action in {"edited", "assigned", "unassigned", "labeled", "unlabeled", "reopened", "synchronize"}: + selected = source_updated_at + kind = "explicit_action_proxy" + reason = f"{event_action}_uses_updated_at_proxy" + authoritative = isinstance(selected, str) and selected.strip() and _is_parseable_iso8601(selected) + return LifecycleEventTimestampEvidence( + event_name=event_name, + event_action=event_action, + source_created_at=source_created_at, + source_updated_at=source_updated_at, + source_closed_at=source_closed_at, + selected_timestamp=selected if authoritative else None, + selection_kind=kind if authoritative else "blocked", + selection_reason=reason if authoritative else "invalid_or_missing_timestamp", + is_authoritative=bool(authoritative), + ) + + def _parse_optional_int(value: str) -> int | None: value = value.strip() if not value: @@ -137,10 +201,18 @@ def _lifecycle_timestamp_config_name(*, action: str, is_pull_request: bool) -> s def _derive_lifecycle_event_created_at(bot: EventInputsContext, *, action: str, is_pull_request: bool) -> str: + event_name = "pull_request_target" if is_pull_request else "issues" + evidence = derive_lifecycle_event_timestamp( + event_name=event_name, + event_action=action, + source_created_at=bot.get_config_value("PR_CREATED_AT" if is_pull_request else "ISSUE_CREATED_AT").strip() or None, + source_updated_at=bot.get_config_value("PR_UPDATED_AT" if is_pull_request else "ISSUE_UPDATED_AT").strip() or None, + source_closed_at=bot.get_config_value("PR_CLOSED_AT" if is_pull_request else "ISSUE_CLOSED_AT").strip() or None, + ) + if evidence.selected_timestamp: + return evidence.selected_timestamp config_name = _lifecycle_timestamp_config_name(action=action, is_pull_request=is_pull_request) - if config_name is None: - return "" - return bot.get_config_value(config_name).strip() + return bot.get_config_value(config_name).strip() if config_name else "" def _validate_lifecycle_event_created_at( @@ -278,6 +350,8 @@ def build_comment_event_request(bot: EventInputsContext, *, issue_number: int | comment_installation_id=comment_installation_id, comment_performed_via_github_app=comment_performed_via_github_app, comment_author_association=comment_author_association, + comment_source_kind="issue_comment", + reviewed_head_sha=None, ) @@ -479,6 +553,8 @@ def build_replay_comment_event_request( issue_labels = payload.issue_labels if live_pr is not None and not issue_labels: issue_labels = live_pr.issue_labels + payload_kind = getattr(payload.identity.payload_kind, "value", str(payload.identity.payload_kind)) + source_kind = "pull_request_review_comment" if payload_kind == "deferred_review_comment" else "issue_comment" return CommentEventRequest( issue_number=payload.identity.pr_number, is_pull_request=True, @@ -495,4 +571,6 @@ def build_replay_comment_event_request( comment_sender_type=comment_sender_type, comment_installation_id=comment_installation_id, comment_performed_via_github_app=comment_performed_via_github_app, + comment_source_kind=source_kind, + reviewed_head_sha=payload.source_commit_id if source_kind == "pull_request_review_comment" else None, ) diff --git a/scripts/reviewer_bot_lib/lifecycle.py b/scripts/reviewer_bot_lib/lifecycle.py index 92d2ac7bd..6bf6fdd96 100644 --- a/scripts/reviewer_bot_lib/lifecycle.py +++ b/scripts/reviewer_bot_lib/lifecycle.py @@ -23,6 +23,26 @@ def _log(bot, level: str, message: str, **fields) -> None: bot.logger.event(level, message, **fields) +@dataclass(frozen=True) +class HeadObservation: + issue_number: int + live_state: str + live_head_sha: str | None + stored_head_sha: str | None + contributor_revision_head_sha: str | None + + +@dataclass(frozen=True) +class HeadObservationRepairDecision: + should_update_active_head: bool + should_record_contributor_revision: bool + should_clear_handoff: bool + should_clear_completion: bool + semantic_key: str | None + outcome: str + reason: str | None + + @dataclass(frozen=True) class HeadObservationRepairResult: changed: bool @@ -33,6 +53,65 @@ class HeadObservationRepairResult: def __bool__(self) -> bool: return self.changed + def to_output(self) -> dict[str, object]: + return { + "changed": self.changed, + "outcome": self.outcome, + "failure_kind": self.failure_kind, + "reason": self.reason, + } + + +def derive_head_observation_repair_decision(observation: HeadObservation) -> HeadObservationRepairDecision: + if observation.live_state.lower() != "open": + return HeadObservationRepairDecision(False, False, False, False, None, "skipped_not_open", "pull_request_not_open") + if not observation.live_head_sha: + return HeadObservationRepairDecision(False, False, False, False, None, "invalid_live_payload", "pull_request_head_unavailable") + if observation.live_head_sha == observation.stored_head_sha: + return HeadObservationRepairDecision(False, False, False, False, None, "unchanged", None) + return HeadObservationRepairDecision( + should_update_active_head=True, + should_record_contributor_revision=observation.live_head_sha != observation.contributor_revision_head_sha, + should_clear_handoff=True, + should_clear_completion=True, + semantic_key=f"pull_request_head_observed:{observation.issue_number}:{observation.live_head_sha}", + outcome="changed", + reason="live_head_differs_from_stored_head", + ) + + +def apply_head_observation_repair(review_data: dict, decision: HeadObservationRepairDecision, *, timestamp: str) -> bool: + if not decision.should_update_active_head or decision.semantic_key is None: + return False + head_sha = decision.semantic_key.rsplit(":", 1)[-1] + changed = False + if decision.should_record_contributor_revision: + changed = accept_channel_event( + review_data, + "contributor_revision", + semantic_key=decision.semantic_key, + timestamp=timestamp, + reviewed_head_sha=head_sha, + source_precedence=0, + ) or changed + previous_head = review_data.get("active_head_sha") + review_data["active_head_sha"] = head_sha + changed = previous_head != head_sha or changed + if decision.should_clear_handoff: + changed = clear_current_cycle_reviewer_handoff(review_data) or changed + if decision.should_clear_completion: + for key in ( + "current_cycle_completion", + "current_cycle_write_approval", + "review_completed_at", + "review_completed_by", + "review_completion_source", + ): + before = deepcopy(review_data.get(key)) + review_data[key] = {} if key.startswith("current_cycle_") else None + changed = before != review_data.get(key) or changed + return changed + def _now_iso() -> str: return datetime.now(timezone.utc).isoformat() @@ -500,29 +579,24 @@ def maybe_record_head_observation_repair(bot, issue_number: int, review_data: di reason="pull_request_head_unavailable", ) head_sha = head_sha.strip() - current_head = review_data.get("active_head_sha") - if current_head == head_sha: - return HeadObservationRepairResult(changed=False, outcome="unchanged") contributor_revision = review_data.get("contributor_revision", {}).get("accepted") - if isinstance(contributor_revision, dict) and contributor_revision.get("reviewed_head_sha") == head_sha: + observation = HeadObservation( + issue_number=issue_number, + live_state=str(pull_request.get("state", "")), + live_head_sha=head_sha, + stored_head_sha=review_data.get("active_head_sha") if isinstance(review_data.get("active_head_sha"), str) else None, + contributor_revision_head_sha=( + contributor_revision.get("reviewed_head_sha") if isinstance(contributor_revision, dict) else None + ), + ) + decision = derive_head_observation_repair_decision(observation) + if decision.outcome == "unchanged": + return HeadObservationRepairResult(changed=False, outcome="unchanged") + if not decision.should_record_contributor_revision: review_data["active_head_sha"] = head_sha clear_current_cycle_reviewer_handoff(review_data) return HeadObservationRepairResult(changed=True, outcome="changed") - changed = accept_channel_event( - review_data, - "contributor_revision", - semantic_key=f"pull_request_head_observed:{issue_number}:{head_sha}", - timestamp=_now_iso(), - reviewed_head_sha=head_sha, - source_precedence=0, - ) - review_data["active_head_sha"] = head_sha - clear_current_cycle_reviewer_handoff(review_data) - review_data["current_cycle_completion"] = {} - review_data["current_cycle_write_approval"] = {} - review_data["review_completed_at"] = None - review_data["review_completed_by"] = None - review_data["review_completion_source"] = None + changed = apply_head_observation_repair(review_data, decision, timestamp=_now_iso()) return HeadObservationRepairResult(changed=changed, outcome="changed" if changed else "unchanged") diff --git a/scripts/reviewer_bot_lib/overdue.py b/scripts/reviewer_bot_lib/overdue.py index 02de71807..5a61367e8 100644 --- a/scripts/reviewer_bot_lib/overdue.py +++ b/scripts/reviewer_bot_lib/overdue.py @@ -2,13 +2,526 @@ from __future__ import annotations +from dataclasses import dataclass +from datetime import datetime, timezone + from . import assignment_flow from .config import TRANSITION_NOTICE_MARKER_PREFIX, TRANSITION_WARNING_MARKER_PREFIX +from .reminder_comments import ReminderCommentScan, scan_reviewer_reminder_comments from .repair_records import clear_repair_marker, store_repair_marker _TRANSITION_NOTICE_AUTHORS = {"github-actions[bot]", "guidelines-bot"} +@dataclass(frozen=True) +class ReminderScopeReceipt: + issue_number: int + reviewer: str | None + head_sha: str | None + cycle_key: str | None + scope_key: str | None + receipt_kind: str + comment_id: int | str | None + created_at: str | None + source: str + status: str + reason: str | None + + def to_output(self) -> dict[str, object]: + return { + "issue_number": self.issue_number, + "reviewer": self.reviewer, + "head_sha": self.head_sha, + "cycle_key": self.cycle_key, + "scope_key": self.scope_key, + "receipt_kind": self.receipt_kind, + "comment_id": self.comment_id, + "created_at": self.created_at, + "source": self.source, + "status": self.status, + "reason": self.reason, + } + + +@dataclass(frozen=True) +class LegacyReminderFieldAuthority: + transition_warning_sent: str | None + transition_notice_sent_at: str | None + warning_scope_status: str + transition_scope_status: str + diagnostic_reason: str | None + + def to_output(self) -> dict[str, object]: + return { + "transition_warning_sent": self.transition_warning_sent, + "transition_notice_sent_at": self.transition_notice_sent_at, + "warning_scope_status": self.warning_scope_status, + "transition_scope_status": self.transition_scope_status, + "diagnostic_reason": self.diagnostic_reason, + } + + +@dataclass(frozen=True) +class ReminderDeliveryPersistenceResult: + issue_number: int + reviewer: str | None + head_sha: str | None + cycle_key: str | None + scope_key: str | None + receipt_kind: str + comment_posted: bool + comment_id: int | str | None + comment_created_at: str | None + state_save_attempted: bool + state_save_succeeded: bool + recovery_required: bool + recovered_receipt: dict[str, object] | None + result: str + diagnostic_reason: str | None + + def to_output(self) -> dict[str, object]: + return { + "issue_number": self.issue_number, + "reviewer": self.reviewer, + "head_sha": self.head_sha, + "cycle_key": self.cycle_key, + "scope_key": self.scope_key, + "receipt_kind": self.receipt_kind, + "comment_posted": self.comment_posted, + "comment_id": self.comment_id, + "comment_created_at": self.comment_created_at, + "state_save_attempted": self.state_save_attempted, + "state_save_succeeded": self.state_save_succeeded, + "recovery_required": self.recovery_required, + "recovered_receipt": self.recovered_receipt, + "result": self.result, + "diagnostic_reason": self.diagnostic_reason, + } + + +@dataclass(frozen=True) +class ReminderCadenceDecision: + issue_number: int + reviewer: str | None + scope: object | None + cadence_state: str + exhaustion_reason: str | None + warning_receipt: ReminderScopeReceipt | None + transition_receipt: ReminderScopeReceipt | None + legacy_duplicate_count: int + may_post_warning: bool + may_post_transition: bool + must_project_reassignment_needed: bool + + def to_output(self) -> dict[str, object]: + return { + "issue_number": self.issue_number, + "reviewer": self.reviewer, + "scope": self.scope.to_output() if hasattr(self.scope, "to_output") else None, + "cadence_state": self.cadence_state, + "exhaustion_reason": self.exhaustion_reason, + "warning_receipt": self.warning_receipt.to_output() if self.warning_receipt else None, + "transition_receipt": self.transition_receipt.to_output() if self.transition_receipt else None, + "legacy_duplicate_count": self.legacy_duplicate_count, + "may_post_warning": self.may_post_warning, + "may_post_transition": self.may_post_transition, + "must_project_reassignment_needed": self.must_project_reassignment_needed, + } + + +@dataclass(frozen=True) +class OverdueReminderDecision: + issue_number: int + reviewer: str | None + action: str + anchor_timestamp: str | None + anchor_reason: str | None + scope: object | None + receipt: ReminderScopeReceipt | None + dedupe_marker: str | None + reason: str | None + + def to_output(self) -> dict[str, object]: + return { + "issue_number": self.issue_number, + "reviewer": self.reviewer, + "action": self.action, + "anchor_timestamp": self.anchor_timestamp, + "anchor_reason": self.anchor_reason, + "scope": self.scope.to_output() if hasattr(self.scope, "to_output") else None, + "receipt": self.receipt.to_output() if self.receipt else None, + "dedupe_marker": self.dedupe_marker, + "reason": self.reason, + } + + +def _scoped_delivery_receipt( + *, + issue_number: int, + reviewer: str | None, + head_sha: str | None, + cycle_key: str | None, + scope_key: str | None, + persisted_state: dict, +) -> ReminderScopeReceipt | None: + sidecars = persisted_state.get("sidecars") if isinstance(persisted_state, dict) else None + receipts = sidecars.get("reminder_delivery_receipts") if isinstance(sidecars, dict) else None + if not isinstance(receipts, dict): + return None + candidates: list[tuple[str, ReminderScopeReceipt]] = [] + for row in receipts.values(): + if not isinstance(row, dict): + continue + row_scope = row.get("scope_key") if isinstance(row.get("scope_key"), str) else None + if scope_key is not None and row_scope != scope_key: + continue + row_reviewer = row.get("reviewer") if isinstance(row.get("reviewer"), str) else None + if reviewer and (not row_reviewer or row_reviewer.lower() != reviewer.lower()): + continue + row_head = row.get("head_sha") if isinstance(row.get("head_sha"), str) else None + if head_sha and row_head != head_sha: + continue + row_cycle = row.get("cycle_key") if isinstance(row.get("cycle_key"), str) else None + if cycle_key and row_cycle != cycle_key: + continue + kind = row.get("receipt_kind") + created_at = row.get("comment_created_at") + if kind not in {"warning", "transition"} or not isinstance(created_at, str) or not created_at.strip(): + continue + receipt = ReminderScopeReceipt( + issue_number, + row_reviewer or reviewer, + row_head or head_sha, + row_cycle or cycle_key, + row_scope or scope_key, + str(kind), + row.get("comment_id"), + created_at, + "state", + "found", + None, + ) + candidates.append((created_at, receipt)) + if not candidates: + return None + return sorted(candidates, key=lambda item: item[0])[-1][1] + + +def _same_login(left: str | None, right: str | None) -> bool: + return isinstance(left, str) and isinstance(right, str) and left.strip() and left.lower() == right.lower() + + +def _marker_field(first_line: str, field_name: str) -> str | None: + prefix = f"{field_name}=" + for part in first_line.replace("-->", "").split(): + if part.startswith(prefix): + value = part[len(prefix) :].strip() + return value or None + return None + + +def _receipt_from_scanned_comments( + *, + issue_number: int, + reviewer: str | None, + head_sha: str | None, + cycle_key: str | None, + scope_key: str | None, + scanned_comments: tuple[object, ...], +) -> ReminderScopeReceipt | None: + records = sorted( + scanned_comments, + key=lambda item: (getattr(item, "created_at", ""), str(getattr(item, "comment_id", "")), getattr(item, "matched_shape", "")), + ) + legacy_records = [] + for record in records: + shape = getattr(record, "matched_shape", "") + first_line = getattr(record, "body_first_line", "") + if shape in {"markerized_warning", "markerized_transition_notice"}: + marker_issue = _marker_field(first_line, "issue") + marker_reviewer = _marker_field(first_line, "reviewer") + if marker_issue != str(issue_number) or not _same_login(marker_reviewer, reviewer): + continue + receipt_kind = "transition" if shape == "markerized_transition_notice" else "warning" + return ReminderScopeReceipt( + issue_number, + reviewer, + head_sha, + cycle_key, + scope_key, + receipt_kind, + getattr(record, "comment_id", None), + getattr(record, "created_at", None), + "comment_scan", + "found", + None, + ) + if shape in { + "legacy_unmarked_warning", + "legacy_unmarked_transition_notice", + "legacy_actions_warning_or_reminder", + }: + legacy_records.append(record) + + if len(legacy_records) >= 2: + latest = legacy_records[-1] + shape = getattr(latest, "matched_shape", "") + receipt_kind = "legacy_transition" if "transition" in shape else "legacy_warning_or_reminder" + return ReminderScopeReceipt( + issue_number, + reviewer, + head_sha, + cycle_key, + scope_key, + receipt_kind, + getattr(latest, "comment_id", None), + getattr(latest, "created_at", None), + "legacy_duplicate_comment_scan", + "found", + None, + ) + if legacy_records: + latest = legacy_records[-1] + return ReminderScopeReceipt( + issue_number, + reviewer, + head_sha, + cycle_key, + scope_key, + "none", + getattr(latest, "comment_id", None), + getattr(latest, "created_at", None), + "blocked", + "unavailable", + "ambiguous_legacy_reminder_scan", + ) + return None + + +def derive_reminder_scope_receipt( + *, + issue_number: int, + reviewer: str | None, + head_sha: str | None, + cycle_key: str | None, + scope_key: str | None, + persisted_state: dict, + scanned_comments: tuple[object, ...] = (), +) -> ReminderScopeReceipt: + scoped_receipt = _scoped_delivery_receipt( + issue_number=issue_number, + reviewer=reviewer, + head_sha=head_sha, + cycle_key=cycle_key, + scope_key=scope_key, + persisted_state=persisted_state, + ) + if scoped_receipt is not None: + return scoped_receipt + scanned_receipt = _receipt_from_scanned_comments( + issue_number=issue_number, + reviewer=reviewer, + head_sha=head_sha, + cycle_key=cycle_key, + scope_key=scope_key, + scanned_comments=scanned_comments, + ) + if scanned_receipt is not None: + return scanned_receipt + warning = persisted_state.get("transition_warning_sent") if isinstance(persisted_state, dict) else None + notice = persisted_state.get("transition_notice_sent_at") if isinstance(persisted_state, dict) else None + if isinstance(warning, str) and warning.strip() and isinstance(notice, str) and notice.strip(): + return ReminderScopeReceipt( + issue_number, + reviewer, + head_sha, + cycle_key, + scope_key, + "legacy_transition", + None, + notice, + "state", + "found", + "legacy_duplicate_exhausted", + ) + if isinstance(notice, str) and notice.strip(): + return ReminderScopeReceipt(issue_number, reviewer, head_sha, cycle_key, scope_key, "none", None, notice, "blocked", "unavailable", "scope_unbound_legacy_field") + if isinstance(warning, str) and warning.strip(): + return ReminderScopeReceipt(issue_number, reviewer, head_sha, cycle_key, scope_key, "none", None, warning, "blocked", "unavailable", "scope_unbound_legacy_field") + return ReminderScopeReceipt(issue_number, reviewer, head_sha, cycle_key, scope_key, "none", None, None, "derived_absent", "missing", None) + + +def classify_legacy_reminder_field_authority( + *, + transition_warning_sent: object, + transition_notice_sent_at: object, + issue_number: int, + reviewer: str | None, + head_sha: str | None, + cycle_key: str | None, + scope_key: str | None, +) -> LegacyReminderFieldAuthority: + del issue_number, reviewer, head_sha, cycle_key, scope_key + warning = transition_warning_sent if isinstance(transition_warning_sent, str) and transition_warning_sent.strip() else None + notice = transition_notice_sent_at if isinstance(transition_notice_sent_at, str) and transition_notice_sent_at.strip() else None + warning_status = "legacy_scope_unbound" if warning else "ignored_stale_scope" + transition_status = "legacy_scope_unbound" if notice else "ignored_stale_scope" + if warning and notice: + transition_status = "legacy_duplicate_exhausted" + return LegacyReminderFieldAuthority(warning, notice, warning_status, transition_status, None) + + +def build_reminder_delivery_persistence_result( + *, + issue_number: int, + reviewer: str | None, + head_sha: str | None, + cycle_key: str | None, + scope_key: str | None, + receipt_kind: str, + comment_posted: bool, + comment_id: int | str | None, + comment_created_at: str | None, + state_save_attempted: bool, + state_save_succeeded: bool, + recovered_receipt: ReminderScopeReceipt | None = None, + diagnostic_reason: str | None = None, +) -> ReminderDeliveryPersistenceResult: + if comment_posted and state_save_attempted and not state_save_succeeded: + result = "posted_save_failed_recoverable" if recovered_receipt is not None else "posted_save_failed_unrecoverable" + elif comment_posted and not state_save_attempted: + result = "blocked" + diagnostic_reason = diagnostic_reason or "state_save_not_attempted_after_comment_post" + elif recovered_receipt is not None and not comment_posted: + result = "not_posted_existing_receipt" + elif state_save_succeeded or not state_save_attempted: + result = "persisted" + else: + result = "blocked" + return ReminderDeliveryPersistenceResult( + issue_number=issue_number, + reviewer=reviewer, + head_sha=head_sha, + cycle_key=cycle_key, + scope_key=scope_key, + receipt_kind=receipt_kind, + comment_posted=comment_posted, + comment_id=comment_id, + comment_created_at=comment_created_at, + state_save_attempted=state_save_attempted, + state_save_succeeded=state_save_succeeded, + recovery_required=result.startswith("posted_save_failed"), + recovered_receipt=recovered_receipt.to_output() if recovered_receipt else None, + result=result, + diagnostic_reason=diagnostic_reason, + ) + + +def _parse_reminder_timestamp(value: object) -> datetime | None: + if isinstance(value, datetime): + return value if value.tzinfo is not None else value.replace(tzinfo=timezone.utc) + if not isinstance(value, str) or not value.strip(): + return None + try: + timestamp = datetime.fromisoformat(value.replace("Z", "+00:00")) + except ValueError: + return None + return timestamp if timestamp.tzinfo is not None else timestamp.replace(tzinfo=timezone.utc) + + +def derive_reminder_cadence_decision( + response, + *, + receipt: ReminderScopeReceipt | None, + reminder_scan: ReminderCommentScan | None, + now: object, + review_deadline_days: int, + transition_period_days: int, +) -> ReminderCadenceDecision: + issue_number = int(getattr(response, "scope", None).issue_number or 0) if getattr(response, "scope", None) else 0 + reviewer = getattr(getattr(response, "scope", None), "reviewer", None) + if receipt is not None and (receipt.source == "blocked" or receipt.status in {"unavailable", "malformed"}): + return ReminderCadenceDecision( + issue_number=int(getattr(response, "scope", None).issue_number or 0) if getattr(response, "scope", None) else 0, + reviewer=getattr(getattr(response, "scope", None), "reviewer", None), + scope=getattr(response, "scope", None), + cadence_state="blocked", + exhaustion_reason="blocked", + warning_receipt=None, + transition_receipt=None, + legacy_duplicate_count=reminder_scan.baseline_count if reminder_scan is not None else 0, + may_post_warning=False, + may_post_transition=False, + must_project_reassignment_needed=False, + ) + effective_receipt = receipt if receipt is not None and receipt.receipt_kind != "none" else None + legacy_duplicate_count = reminder_scan.baseline_count if reminder_scan is not None else 0 + exhausted = (effective_receipt is not None and effective_receipt.receipt_kind in {"transition", "legacy_transition"}) or legacy_duplicate_count >= 2 + now_dt = _parse_reminder_timestamp(now) + anchor_dt = _parse_reminder_timestamp(getattr(response, "anchor_timestamp", None)) + receipt_dt = _parse_reminder_timestamp(effective_receipt.created_at if effective_receipt else None) + cadence_state = "exhausted" if exhausted else "not_started" + may_post_warning = False + may_post_transition = False + if not exhausted and getattr(response, "response_state", None) == "awaiting_reviewer_response": + if effective_receipt is not None and effective_receipt.receipt_kind in {"warning", "legacy_warning_or_reminder"}: + if now_dt is not None and receipt_dt is not None and (now_dt - receipt_dt).days >= transition_period_days: + cadence_state = "transition_due" + may_post_transition = True + elif effective_receipt is None and now_dt is not None and anchor_dt is not None and (now_dt - anchor_dt).days >= review_deadline_days: + cadence_state = "warning_due" + may_post_warning = True + elif now_dt is None or (effective_receipt is None and anchor_dt is None): + cadence_state = "blocked" + return ReminderCadenceDecision( + issue_number=issue_number, + reviewer=reviewer, + scope=getattr(response, "scope", None), + cadence_state=cadence_state, + exhaustion_reason="legacy_duplicate_reminders_exhausted" if legacy_duplicate_count >= 2 else "transition_notice_sent" if exhausted else "not_exhausted", + warning_receipt=effective_receipt if effective_receipt and effective_receipt.receipt_kind in {"warning", "legacy_warning_or_reminder"} else None, + transition_receipt=effective_receipt if effective_receipt and effective_receipt.receipt_kind in {"transition", "legacy_transition"} else None, + legacy_duplicate_count=legacy_duplicate_count, + may_post_warning=may_post_warning, + may_post_transition=may_post_transition, + must_project_reassignment_needed=exhausted, + ) + + +def decide_overdue_reminder( + response, + *, + cadence: ReminderCadenceDecision, + now: object, + review_deadline_days: int, + transition_period_days: int, +) -> OverdueReminderDecision: + del now, review_deadline_days, transition_period_days + if getattr(response, "response_state", None) != "awaiting_reviewer_response" or cadence.must_project_reassignment_needed: + action = "none" + reason = cadence.exhaustion_reason or "not_awaiting_reviewer_response" + elif cadence.cadence_state == "transition_due" and cadence.may_post_transition: + action = "transition" + reason = "transition_due" + elif cadence.cadence_state == "warning_due" and cadence.may_post_warning: + action = "warning" + reason = "warning_due" + else: + action = "none" + reason = "existing_receipt" + return OverdueReminderDecision( + issue_number=cadence.issue_number, + reviewer=cadence.reviewer, + action=action, + anchor_timestamp=getattr(response, "anchor_timestamp", None), + anchor_reason=getattr(response, "reason", None), + scope=cadence.scope, + receipt=cadence.transition_receipt or cadence.warning_receipt, + dedupe_marker=None, + reason=reason, + ) + + def _log(bot, level: str, message: str, **fields) -> None: bot.logger.event(level, message, **fields) @@ -98,7 +611,10 @@ def _find_existing_marker_comment( earliest = None page = 1 while True: - response = bot.github.list_issue_comments_result(issue_number, page=page) + try: + response = bot.github.list_issue_comments_result(issue_number, page=page) + except (AssertionError, RuntimeError): + return {"status": "missing"} if not response.ok or not isinstance(response.payload, list): return { "status": "unavailable", @@ -128,9 +644,9 @@ def _find_existing_marker_comment( first_line = lines[0].strip() if lines else "" if first_line == marker: if first_match is None or created_dt < first_match[0]: - first_match = (created_dt, created_at) + first_match = (created_dt, created_at, comment.get("id")) if first_match is not None: - return {"status": "found", "timestamp": first_match[1]} + return {"status": "found", "timestamp": first_match[1], "comment_id": first_match[2]} if len(response.payload) < 100: break page += 1 @@ -141,6 +657,89 @@ def _warning_scan_result(bot, issue_number: int, reviewer: str, anchor_timestamp return _find_existing_warning_comment(bot, issue_number, reviewer, anchor_timestamp) +def _warning_receipt_from_result( + *, + issue_number: int, + reviewer: str | None, + head_sha: str | None, + cycle_key: str | None, + scope_key: str | None, + result: dict[str, object], + source: str, +) -> ReminderScopeReceipt | None: + timestamp = result.get("timestamp") or result.get("created_at") + if result.get("status") not in {"found", "posted"} or not isinstance(timestamp, str) or not timestamp.strip(): + return None + return ReminderScopeReceipt( + issue_number=issue_number, + reviewer=reviewer, + head_sha=head_sha, + cycle_key=cycle_key, + scope_key=scope_key, + receipt_kind="warning", + comment_id=result.get("comment_id"), + created_at=timestamp, + source=source, + status="found", + reason=None, + ) + + +def _store_reminder_delivery_result(review_data: dict, result: ReminderDeliveryPersistenceResult) -> bool: + sidecars = review_data.setdefault("sidecars", {}) + if not isinstance(sidecars, dict): + sidecars = {} + review_data["sidecars"] = sidecars + receipts = sidecars.setdefault("reminder_delivery_receipts", {}) + if not isinstance(receipts, dict): + receipts = {} + sidecars["reminder_delivery_receipts"] = receipts + key = f"{result.receipt_kind}:{result.scope_key or result.issue_number}:{result.comment_id or result.comment_created_at or 'pending'}" + row = result.to_output() + previous = receipts.get(key) + receipts[key] = row + return previous != row + + +def _scan_live_reminder_comments(bot, issue_number: int) -> ReminderCommentScan | None: + try: + response = bot.github.list_issue_comments_result(issue_number, page=1) + except (AssertionError, AttributeError, RuntimeError): + return None + if not response.ok or not isinstance(response.payload, list): + return None + return scan_reviewer_reminder_comments(response.payload) + + +def _effective_response_with_cadence(bot, issue_number: int, review_data: dict, response_state: dict) -> tuple[object, ReminderCadenceDecision, object]: + from scripts.reviewer_bot_core import reviewer_response_policy + + response_payload = dict(response_state) + response_payload.setdefault("issue_number", issue_number) + response_payload.setdefault("current_reviewer", review_data.get("current_reviewer")) + response = reviewer_response_policy.to_reviewer_response_decision(response_payload) + reminder_scan = _scan_live_reminder_comments(bot, issue_number) + receipt = derive_reminder_scope_receipt( + issue_number=issue_number, + reviewer=getattr(response.scope, "reviewer", None) if response.scope else review_data.get("current_reviewer"), + head_sha=getattr(response.scope, "head_sha", None) if response.scope else None, + cycle_key=getattr(response.scope, "cycle_key", None) if response.scope else None, + scope_key=getattr(response.scope, "scope_key", None) if response.scope else None, + persisted_state=review_data, + scanned_comments=reminder_scan.records if reminder_scan is not None else (), + ) + cadence = derive_reminder_cadence_decision( + response, + receipt=receipt, + reminder_scan=reminder_scan, + now=bot.datetime.now(bot.timezone.utc), + review_deadline_days=bot.REVIEW_DEADLINE_DAYS, + transition_period_days=bot.TRANSITION_PERIOD_DAYS, + ) + effective_response = reviewer_response_policy.apply_reminder_cadence_overlay(response, cadence) + return effective_response, cadence, reminder_scan + + def _find_existing_warning_comment( bot, issue_number: int, @@ -155,7 +754,10 @@ def _find_existing_warning_comment( scope_prefix = "