diff --git a/docs/automation/review-agent-comment-invocation.md b/docs/automation/review-agent-comment-invocation.md index 3d2ca496d..cc8f8c58c 100644 --- a/docs/automation/review-agent-comment-invocation.md +++ b/docs/automation/review-agent-comment-invocation.md @@ -1,6 +1,6 @@ # Review-agent comment invocation -Updated: 2026-08-06 +Updated: 2026-08-19 ## Purpose @@ -28,6 +28,8 @@ Wrapper workflows use the verified key in their non-cancelling concurrency group Target-repository acknowledgement comments and reactions are user-experience signals only. They are not dispatch authority because repository writers, bot identities, or credential rotation could otherwise forge or invalidate a marker. A failed acknowledgement cannot cause completed agent work to be redispatched. +When a live claim exists without a visible receipt comment, the router republishes the acknowledgement without forwarding the request again; reaction failures are warnings and do not block the durable comment. + A user or fine-grained token enumerates organization repositories. When the OpenCode GitHub App installation token is the available credential, the sweep instead uses GitHub's installation-repositories endpoint, which returns only repositories accessible to that installation. This avoids depending on an organization-issues endpoint whose documented fine-grained token support is user-token-oriented. This preserves the central MSA boundary without copying privileged workflow code into every product repository. diff --git a/scripts/ci/agent_mention_router.py b/scripts/ci/agent_mention_router.py index 2b5453139..e165cb4f8 100644 --- a/scripts/ci/agent_mention_router.py +++ b/scripts/ci/agent_mention_router.py @@ -464,6 +464,16 @@ def dispatch_request( ) return handles + acknowledgement_cache_key = ( + f"acknowledgement:{request.repository}:{request.pull_request_number}:" + f"{request.pull_request_head_sha}:{request.comment_id}" + ) + if ( + ledger_artifact_cache is not None + and ledger_artifact_cache.get(acknowledgement_cache_key) + ): + return () + existing = dispatched_agents( request, dispatch_client, @@ -472,7 +482,10 @@ def dispatch_request( ) missing = tuple(agent for agent in dispatchable if agent not in existing) handles = tuple(f"@{agent}" for agent in missing) - if not missing: + existing_handles = tuple( + f"@{agent}" for agent in dispatchable if agent in existing + ) + if not missing and not existing: if rejected: print( "Rejected agent mention without target mutation " @@ -501,18 +514,24 @@ def dispatch_request( ledger_artifact_cache[agent_ledger_artifact_name(request, agent)] = True target_api = f"repos/{request.repository}" - target_client.request( - [ - f"{target_api}/issues/comments/{request.comment_id}/reactions", - "-X", - "POST", - ], - input_payload={"content": "eyes"}, - ) - status_parts = [f"Queued {' and '.join(handles)}"] - existing_handles = tuple( - f"@{agent}" for agent in dispatchable if agent in existing - ) + try: + target_client.request( + [ + f"{target_api}/issues/comments/{request.comment_id}/reactions", + "-X", + "POST", + ], + input_payload={"content": "eyes"}, + ) + except Exception as exc: # noqa: BLE001 - acknowledgement is cosmetic + message = " ".join(str(exc).split()) or exc.__class__.__name__ + print( + "::warning::Agent mention acknowledgement reaction failed; " + f"durable dispatch state is preserved: {message[:1000]}" + ) + status_parts: list[str] = [] + if handles: + status_parts.append(f"Queued {' and '.join(handles)}") if existing_handles: status_parts.append( f"Already queued {' and '.join(existing_handles)} on this exact request" @@ -538,6 +557,8 @@ def dispatch_request( ], input_payload={"body": acknowledgement}, ) + if ledger_artifact_cache is not None: + ledger_artifact_cache[acknowledgement_cache_key] = True return handles diff --git a/tests/test_agent_mention_acknowledgement_recovery.py b/tests/test_agent_mention_acknowledgement_recovery.py new file mode 100644 index 000000000..840e69a06 --- /dev/null +++ b/tests/test_agent_mention_acknowledgement_recovery.py @@ -0,0 +1,156 @@ +"""Regression tests for post-dispatch mention acknowledgement recovery.""" + +from __future__ import annotations + +import importlib.util +import sys +from pathlib import Path +from types import ModuleType + +import pytest + +ROOT = Path(__file__).resolve().parents[1] +MODULE_PATH = ROOT / "scripts" / "ci" / "agent_mention_router.py" + + +def load_module() -> ModuleType: + """Load the central mention router from its script path.""" + + module_name = "agent_mention_router_acknowledgement_recovery" + spec = importlib.util.spec_from_file_location(module_name, MODULE_PATH) + assert spec and spec.loader + module = importlib.util.module_from_spec(spec) + sys.modules[module_name] = module + spec.loader.exec_module(module) + return module + + +def request(module: ModuleType): + """Build one exact trusted OpenCode mention request.""" + + return module.MentionRequest( + repository="ContextualWisdomLab/.github", + pull_request_number=1099, + pull_request_head_sha="a" * 40, + pull_request_base_branch="main", + comment_id=91, + actor="maintainer", + agents=("opencode-agent",), + pull_request_base_sha="b" * 40, + ) + + +class FakeClient: + """Capture API traffic while simulating ledger and UX failures.""" + + def __init__( + self, + *, + existing_claim: bool = False, + fail_reaction: bool = False, + fail_comment: bool = False, + ) -> None: + """Initialize deterministic response and failure controls.""" + + self.existing_claim = existing_claim + self.fail_reaction = fail_reaction + self.fail_comment = fail_comment + self.calls: list[tuple[list[str], dict | None]] = [] + + def request(self, args, *, input_payload=None): + """Record one request and return or raise the configured outcome.""" + + arguments = list(args) + self.calls.append((arguments, input_payload)) + endpoint = arguments[0] + if endpoint.endswith("/actions/artifacts"): + if not self.existing_claim: + return {"total_count": 0, "artifacts": []} + name = next( + value.removeprefix("name=") + for value in arguments + if value.startswith("name=") + ) + return { + "total_count": 1, + "artifacts": [{"id": 17, "name": name, "expired": False}], + } + if endpoint.endswith("/reactions") and self.fail_reaction: + raise RuntimeError("Resource not accessible by integration (HTTP 403)") + if endpoint.endswith("/issues/1099/comments") and self.fail_comment: + raise RuntimeError("comment publication failed") + return None + + +def dispatch_mutations(client: FakeClient) -> list[tuple[list[str], dict | None]]: + """Return only repository-dispatch mutation calls.""" + + return [call for call in client.calls if call[0][0].endswith("/dispatches")] + + +def acknowledgement_comments(client: FakeClient) -> list[dict]: + """Return published target-PR acknowledgement payloads.""" + + return [ + payload + for args, payload in client.calls + if args[0].endswith("/issues/1099/comments") and payload is not None + ] + + +def test_existing_durable_claim_heals_missing_acknowledgement() -> None: + """A ledgered invocation is acknowledged without a duplicate dispatch.""" + + module = load_module() + central = FakeClient(existing_claim=True) + target = FakeClient() + + assert module.dispatch_request( + request(module), + target_client=target, + dispatch_client=central, + opencode_allowlist=frozenset({"ContextualWisdomLab/.github"}), + ) == () + + assert dispatch_mutations(central) == [] + comments = acknowledgement_comments(target) + assert len(comments) == 1 + assert "Already queued @opencode-agent on this exact request" in comments[0]["body"] + assert "cwl-agent-mention-receipt:91" in comments[0]["body"] + + +def test_reaction_failure_does_not_hide_successful_dispatch(capsys) -> None: + """A cosmetic reaction 403 cannot suppress the durable acknowledgement.""" + + module = load_module() + central = FakeClient() + target = FakeClient(fail_reaction=True) + + assert module.dispatch_request( + request(module), + target_client=target, + dispatch_client=central, + opencode_allowlist=frozenset({"ContextualWisdomLab/.github"}), + ) == ("@opencode-agent",) + + assert len(dispatch_mutations(central)) == 1 + assert len(acknowledgement_comments(target)) == 1 + assert "::warning::" in capsys.readouterr().out + + +def test_acknowledgement_comment_failure_remains_visible() -> None: + """A missing durable receipt still fails so a later sweep can repair it.""" + + module = load_module() + central = FakeClient() + target = FakeClient(fail_comment=True) + + with pytest.raises(RuntimeError, match="comment publication failed"): + module.dispatch_request( + request(module), + target_client=target, + dispatch_client=central, + opencode_allowlist=frozenset({"ContextualWisdomLab/.github"}), + ) + + assert len(dispatch_mutations(central)) == 1 diff --git a/tests/test_agent_mention_idempotency.py b/tests/test_agent_mention_idempotency.py index 499730a22..fc112116d 100644 --- a/tests/test_agent_mention_idempotency.py +++ b/tests/test_agent_mention_idempotency.py @@ -320,13 +320,12 @@ def test_reaction_or_ack_failure_cannot_redispatch_completed_agents() -> None: mention_request = request(module) central = ArtifactAwareClient() failing_target = ArtifactAwareClient(fail_target_call=1) - with pytest.raises(RuntimeError, match="target call"): - module.dispatch_request( - mention_request, - target_client=failing_target, - dispatch_client=central, - opencode_allowlist=frozenset({mention_request.repository}), - ) + assert module.dispatch_request( + mention_request, + target_client=failing_target, + dispatch_client=central, + opencode_allowlist=frozenset({mention_request.repository}), + ) == ("@cwl-noema-review", "@opencode-agent") assert dispatch_events(central) == [ "agent-mention-noema", "agent-mention-opencode", @@ -348,4 +347,4 @@ def test_reaction_or_ack_failure_cannot_redispatch_completed_agents() -> None: opencode_allowlist=frozenset({mention_request.repository}), ) == () assert dispatch_events(retry) == [] - assert retry_target.calls == [] + assert len(retry_target.calls) == 2 diff --git a/tests/test_agent_mention_rejection_idempotency.py b/tests/test_agent_mention_rejection_idempotency.py index 843454f3d..2e12e3867 100644 --- a/tests/test_agent_mention_rejection_idempotency.py +++ b/tests/test_agent_mention_rejection_idempotency.py @@ -64,3 +64,29 @@ def test_rejected_only_request_is_mutation_free() -> None: ) == () assert target.calls == [] assert central.calls == [] + + +def test_empty_request_is_mutation_free() -> None: + """An already-filtered request does not emit a rejection or mutate GitHub.""" + + module = load_module() + request = module.MentionRequest( + "ContextualWisdomLab/example", + 17, + "a" * 40, + "main", + 91, + "maintainer", + (), + ) + target = FakeClient() + central = FakeClient() + + assert module.dispatch_request( + request, + target_client=target, + dispatch_client=central, + opencode_allowlist=frozenset(), + ) == () + assert target.calls == [] + assert central.calls == []