diff --git a/.github/workflows/reviewer-bot-issue-comment-direct.yml b/.github/workflows/reviewer-bot-issue-comment-direct.yml index 2e389a7b1..f9ac7cb9c 100644 --- a/.github/workflows/reviewer-bot-issue-comment-direct.yml +++ b/.github/workflows/reviewer-bot-issue-comment-direct.yml @@ -51,6 +51,7 @@ 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 }} @@ -58,6 +59,7 @@ jobs: 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 }} diff --git a/.github/workflows/reviewer-bot-pr-metadata.yml b/.github/workflows/reviewer-bot-pr-metadata.yml index 6386af45d..70e17b56c 100644 --- a/.github/workflows/reviewer-bot-pr-metadata.yml +++ b/.github/workflows/reviewer-bot-pr-metadata.yml @@ -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' diff --git a/scripts/reviewer_bot_lib/bootstrap_runtime.py b/scripts/reviewer_bot_lib/bootstrap_runtime.py index a1b1c200e..8d1865f2a 100644 --- a/scripts/reviewer_bot_lib/bootstrap_runtime.py +++ b/scripts/reviewer_bot_lib/bootstrap_runtime.py @@ -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) diff --git a/tests/contract/reviewer_bot/test_adapter_contract.py b/tests/contract/reviewer_bot/test_adapter_contract.py index ee07cb737..88a61f5fd 100644 --- a/tests/contract/reviewer_bot/test_adapter_contract.py +++ b/tests/contract/reviewer_bot/test_adapter_contract.py @@ -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() @@ -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" diff --git a/tests/contract/reviewer_bot/test_fake_runtime_contract.py b/tests/contract/reviewer_bot/test_fake_runtime_contract.py index e00117fa4..4b9474d0f 100644 --- a/tests/contract/reviewer_bot/test_fake_runtime_contract.py +++ b/tests/contract/reviewer_bot/test_fake_runtime_contract.py @@ -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" ) diff --git a/tests/contract/reviewer_bot/test_workflow_files.py b/tests/contract/reviewer_bot/test_workflow_files.py index c44bb14ce..c96be44b3 100644 --- a/tests/contract/reviewer_bot/test_workflow_files.py +++ b/tests/contract/reviewer_bot/test_workflow_files.py @@ -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 diff --git a/tests/fixtures/equivalence/runtime_surface/triple_inventory.json b/tests/fixtures/equivalence/runtime_surface/triple_inventory.json index 9a883ef78..59ee814f3 100644 --- a/tests/fixtures/equivalence/runtime_surface/triple_inventory.json +++ b/tests/fixtures/equivalence/runtime_surface/triple_inventory.json @@ -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, diff --git a/tests/integration/reviewer_bot/test_app_execution.py b/tests/integration/reviewer_bot/test_app_execution.py index 64c7f974b..f8e944b31 100644 --- a/tests/integration/reviewer_bot/test_app_execution.py +++ b/tests/integration/reviewer_bot/test_app_execution.py @@ -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 @@ -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")