Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .github/workflows/reviewer-bot-issue-comment-direct.yml
Original file line number Diff line number Diff line change
Expand Up @@ -51,13 +51,15 @@ jobs:
EVENT_NAME: issue_comment
EVENT_ACTION: created
ISSUE_NUMBER: ${{ github.event.issue.number }}
IS_PULL_REQUEST: 'false'
ISSUE_STATE: ${{ github.event.issue.state }}
ISSUE_TITLE: ${{ github.event.issue.title }}
ISSUE_AUTHOR: ${{ github.event.issue.user.login }}
ISSUE_HTML_URL: ${{ github.event.issue.html_url }}
ISSUE_LABELS: ${{ toJson(github.event.issue.labels.*.name) }}
COMMENT_BODY: ${{ github.event.comment.body }}
COMMENT_AUTHOR: ${{ github.event.comment.user.login }}
COMMENT_AUTHOR_ID: ${{ github.event.comment.user.id }}
COMMENT_ID: ${{ github.event.comment.id }}
COMMENT_CREATED_AT: ${{ github.event.comment.created_at }}
COMMENT_USER_TYPE: ${{ github.event.comment.user.type }}
Expand Down
1 change: 1 addition & 0 deletions .github/workflows/reviewer-bot-pr-metadata.yml
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ jobs:
EVENT_NAME: pull_request_target
EVENT_ACTION: ${{ github.event.action }}
ISSUE_NUMBER: ${{ github.event.pull_request.number }}
LABEL_NAME: ${{ github.event.label.name }}
ISSUE_AUTHOR: ${{ github.event.pull_request.user.login }}
ISSUE_LABELS: ${{ toJson(github.event.pull_request.labels.*.name) }}
IS_PULL_REQUEST: 'true'
Expand Down
3 changes: 3 additions & 0 deletions scripts/reviewer_bot_lib/bootstrap_runtime.py
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,9 @@ def _runtime(self):
def normalize_lock_metadata(self, lock_meta):
return state_store.normalize_lock_metadata(lock_meta)

def assert_lock_held(self, operation):
return state_store.assert_lock_held(self._runtime(), operation)

def parse_iso8601_timestamp(self, value):
return state_store.parse_iso8601_timestamp(value)

Expand Down
17 changes: 17 additions & 0 deletions tests/contract/reviewer_bot/test_adapter_contract.py
Original file line number Diff line number Diff line change
Expand Up @@ -543,9 +543,20 @@ def test_bootstrap_runtime_wires_explicit_adapter_services():
assert hasattr(runtime.adapters.review_state, "ensure_review_entry")
assert hasattr(runtime.adapters.commands, "handle_pass_command")
assert hasattr(runtime.adapters.queue, "get_next_reviewer")
assert hasattr(runtime.adapters.state_lock, "assert_lock_held")
assert hasattr(runtime.adapters.state_lock, "render_state_issue_body")


def test_bootstrapped_runtime_state_lock_assertion_delegates_through_adapter_surface():
runtime = reviewer_bot._runtime_bot()

with pytest.raises(RuntimeError, match="Mutating path reached without lease lock: contract-check"):
runtime.assert_lock_held("contract-check")

runtime.ACTIVE_LEASE_CONTEXT = object()
runtime.assert_lock_held("contract-check")


def test_status_label_sync_contract_stays_on_workflow_adapter_surface():
runtime = reviewer_bot._runtime_bot()

Expand Down Expand Up @@ -586,6 +597,12 @@ def test_f2a_runtime_surface_inventory_matches_bootstrap_adapter_examples():
capabilities = {entry["capability"]: entry for entry in inventory["capability_triples"]}

assert capabilities["comment-event dispatch"]["bootstrap_adapter"].endswith("handle_comment_event")
assert capabilities["state-lock assertion delegation"]["runtime_forwarder"].endswith(
"assert_lock_held"
)
assert capabilities["state-lock assertion delegation"]["bootstrap_adapter"].endswith(
"assert_lock_held"
)
assert "workflow-run dispatch" not in capabilities
assert capabilities["sync status labels"]["bootstrap_adapter"].endswith(
"sync_status_labels_for_items"
Expand Down
3 changes: 3 additions & 0 deletions tests/contract/reviewer_bot/test_fake_runtime_contract.py
Original file line number Diff line number Diff line change
Expand Up @@ -334,6 +334,9 @@ def test_f2a_runtime_surface_inventory_matches_fake_runtime_branch_examples():
capabilities = {entry["capability"]: entry for entry in inventory["capability_triples"]}

assert capabilities["comment-event dispatch"]["fake_runtime_branch"].endswith("handle_comment_event")
assert capabilities["state-lock assertion delegation"]["fake_runtime_branch"].endswith(
"assert_lock_held"
)
assert capabilities["privileged accept-no-fls-changes execution"]["fake_runtime_branch"].endswith(
"handle_accept_no_fls_changes_command"
)
Expand Down
15 changes: 15 additions & 0 deletions tests/contract/reviewer_bot/test_workflow_files.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,21 @@ def test_issue_comment_direct_workflow_exports_issue_state():
)
assert "ISSUE_STATE: ${{ github.event.issue.state }}" in workflow_text


def test_issue_comment_direct_workflow_exports_retained_request_inputs():
workflow_text = Path(".github/workflows/reviewer-bot-issue-comment-direct.yml").read_text(
encoding="utf-8"
)
assert "IS_PULL_REQUEST: 'false'" in workflow_text
assert "COMMENT_AUTHOR_ID: ${{ github.event.comment.user.id }}" in workflow_text


def test_pr_metadata_workflow_exports_label_name_for_labeled_path():
workflow_text = Path(".github/workflows/reviewer-bot-pr-metadata.yml").read_text(
encoding="utf-8"
)
assert "LABEL_NAME: ${{ github.event.label.name }}" in workflow_text

def test_pr_comment_router_workflow_builds_payload_inline_without_bot_src_root():
workflow = Path(".github/workflows/reviewer-bot-pr-comment-router.yml").read_text(encoding="utf-8")
assert "build_pr_comment_observer_payload" not in workflow
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,13 @@
"fake_runtime_branch": "tests/fixtures/fake_runtime.py:sync_status_labels_for_items",
"classification": "retained final surface"
},
{
"capability": "state-lock assertion delegation",
"runtime_forwarder": "scripts/reviewer_bot_lib/runtime.py:assert_lock_held",
"bootstrap_adapter": "scripts/reviewer_bot_lib/bootstrap_runtime.py:_BootstrapStateLockAdapterServices.assert_lock_held",
"fake_runtime_branch": "tests/fixtures/fake_runtime.py:assert_lock_held",
"classification": "retained final surface"
},
{
"capability": "privileged branch lookup",
"runtime_forwarder": null,
Expand Down
104 changes: 103 additions & 1 deletion tests/integration/reviewer_bot/test_app_execution.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@

import pytest

from scripts.reviewer_bot_lib import reconcile
from scripts import reviewer_bot
from scripts.reviewer_bot_lib import event_inputs, reconcile
from tests.fixtures.app_harness import AppHarness
from tests.fixtures.reviewer_bot import make_state, make_tracked_review_state

Expand Down Expand Up @@ -184,6 +185,107 @@ def test_execute_run_returns_failure_for_invalid_workflow_run_context(monkeypatc
assert result.state_changed is False


def test_bootstrapped_runtime_executes_direct_issue_comment_path_with_strict_request_inputs(monkeypatch):
runtime = reviewer_bot._runtime_bot()
state = make_state()
seen = {}

def acquire_lock():
runtime.ACTIVE_LEASE_CONTEXT = object()
return runtime.ACTIVE_LEASE_CONTEXT

def release_lock():
runtime.ACTIVE_LEASE_CONTEXT = None
return True

def handle_comment_event(current_state):
assert current_state is state
seen["request"] = event_inputs.build_comment_event_request(runtime)
runtime.collect_touched_item(42)
return True

monkeypatch.setattr(runtime.locks, "acquire", acquire_lock)
monkeypatch.setattr(runtime.locks, "release", release_lock)
monkeypatch.setattr(runtime.state_store, "load_state", lambda *, fail_on_unavailable=False: state)
monkeypatch.setattr(runtime.state_store, "save_state", lambda current_state: True)
monkeypatch.setattr(runtime.adapters.workflow, "process_pass_until_expirations", lambda current_state: (current_state, []))
monkeypatch.setattr(runtime.adapters.workflow, "sync_members_with_queue", lambda current_state: (current_state, []))
monkeypatch.setattr(runtime.adapters.workflow, "sync_status_labels_for_items", lambda current_state, issue_numbers: False)
monkeypatch.setattr(runtime.handlers, "handle_comment_event", handle_comment_event)
monkeypatch.setenv("EVENT_NAME", "issue_comment")
monkeypatch.setenv("EVENT_ACTION", "created")
monkeypatch.setenv("ISSUE_NUMBER", "42")
monkeypatch.setenv("IS_PULL_REQUEST", "false")
monkeypatch.setenv("ISSUE_STATE", "open")
monkeypatch.setenv("ISSUE_AUTHOR", "dana")
monkeypatch.setenv("ISSUE_LABELS", '["triage"]')
monkeypatch.setenv("COMMENT_ID", "100")
monkeypatch.setenv("COMMENT_AUTHOR", "alice")
monkeypatch.setenv("COMMENT_AUTHOR_ID", "200")
monkeypatch.setenv("COMMENT_BODY", "hello")
monkeypatch.setenv("COMMENT_CREATED_AT", "2026-04-13T04:30:00Z")
monkeypatch.delenv("COMMENT_SOURCE_EVENT_KEY", raising=False)
monkeypatch.setenv("COMMENT_USER_TYPE", "User")
monkeypatch.setenv("COMMENT_SENDER_TYPE", "User")
monkeypatch.delenv("COMMENT_INSTALLATION_ID", raising=False)
monkeypatch.setenv("COMMENT_PERFORMED_VIA_GITHUB_APP", "false")

context = reviewer_bot.build_event_context(runtime)
result = reviewer_bot.execute_run(context, runtime)

assert result.exit_code == 0
assert result.state_changed is True
assert seen["request"].is_pull_request is False
assert seen["request"].comment_author_id == 200
assert seen["request"].comment_source_event_key == "issue_comment:100"
assert runtime.ACTIVE_LEASE_CONTEXT is None


def test_bootstrapped_runtime_executes_pr_metadata_closed_dispatch_path(monkeypatch):
runtime = reviewer_bot._runtime_bot()
state = make_state()
calls = []

def acquire_lock():
runtime.ACTIVE_LEASE_CONTEXT = object()
return runtime.ACTIVE_LEASE_CONTEXT

def release_lock():
runtime.ACTIVE_LEASE_CONTEXT = None
return True

def handle_closed_event(current_state):
calls.append(current_state)
return True

monkeypatch.setattr(runtime.locks, "acquire", acquire_lock)
monkeypatch.setattr(runtime.locks, "release", release_lock)
monkeypatch.setattr(runtime.state_store, "load_state", lambda *, fail_on_unavailable=False: state)
monkeypatch.setattr(runtime.state_store, "save_state", lambda current_state: True)
monkeypatch.setattr(runtime.adapters.workflow, "process_pass_until_expirations", lambda current_state: (current_state, []))
monkeypatch.setattr(runtime.adapters.workflow, "sync_members_with_queue", lambda current_state: (current_state, []))
monkeypatch.setattr(runtime.adapters.workflow, "sync_status_labels_for_items", lambda current_state, issue_numbers: False)
monkeypatch.setattr(runtime.handlers, "handle_closed_event", handle_closed_event)
monkeypatch.setenv("EVENT_NAME", "pull_request_target")
monkeypatch.setenv("EVENT_ACTION", "closed")
monkeypatch.setenv("ISSUE_NUMBER", "42")
monkeypatch.setenv("IS_PULL_REQUEST", "true")
monkeypatch.setenv("ISSUE_AUTHOR", "dana")
monkeypatch.setenv("ISSUE_LABELS", '["triage"]')
monkeypatch.setenv("PR_HEAD_SHA", "head-1")
monkeypatch.setenv("EVENT_CREATED_AT", "2026-04-13T04:31:00Z")

context = reviewer_bot.build_event_context(runtime)
result = reviewer_bot.execute_run(context, runtime)

assert context.event_name == "pull_request_target"
assert context.event_action == "closed"
assert calls == [state]
assert result.exit_code == 0
assert result.state_changed is True
assert runtime.ACTIVE_LEASE_CONTEXT is None


def test_d4a_app_branch_to_phase_map_is_frozen_pre_edit():
app_text = Path("scripts/reviewer_bot_lib/app.py").read_text(encoding="utf-8")

Expand Down
Loading