diff --git a/.github/workflows/agent-mention-router.yml b/.github/workflows/agent-mention-router.yml index 43fb16397..99084e31a 100644 --- a/.github/workflows/agent-mention-router.yml +++ b/.github/workflows/agent-mention-router.yml @@ -25,8 +25,8 @@ jobs: || contains(github.event.comment.body, '@opencode-agent') ) concurrency: - group: review-agent-mention-router-local-${{ github.repository }} - queue: max + group: review-agent-mention-router-local-${{ github.repository }}-${{ github.event.comment.id }} + cancel-in-progress: false runs-on: ubuntu-24.04 timeout-minutes: 5 permissions: @@ -38,7 +38,7 @@ jobs: FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: true GH_TOKEN: ${{ github.token }} TARGET_REPOSITORY_TOKEN: ${{ github.token }} - AGENT_DISPATCH_TOKEN: ${{ github.token }} + AGENT_DISPATCH_TOKEN: ${{ secrets.PR_REVIEW_MERGE_TOKEN || secrets.OPENCODE_APPROVE_TOKEN }} OPENCODE_REPOSITORY_DISPATCH_TARGETS: ${{ vars.OPENCODE_REPOSITORY_DISPATCH_TARGETS }} steps: - name: Check out trusted default-branch router @@ -61,9 +61,13 @@ jobs: "$SOURCE_EVENT_PATH" >"${RUNNER_TEMP}/agent-mention-event.json" - name: Route trusted local agent mention - run: >- - python3 -u scripts/ci/agent_mention_router.py - --event-path "${RUNNER_TEMP}/agent-mention-event.json" + run: | + set -euo pipefail + if [ -z "${AGENT_DISPATCH_TOKEN:-}" ]; then + echo "::warning::No configured review dispatch credential; the mention remains available for the scheduled sweep." + exit 0 + fi + python3 -u scripts/ci/agent_mention_router.py --event-path "${RUNNER_TEMP}/agent-mention-event.json" sweep-organization-agent-mentions: if: >- @@ -158,7 +162,6 @@ jobs: env: PR_REVIEW_MERGE_TOKEN: ${{ secrets.PR_REVIEW_MERGE_TOKEN }} OPENCODE_APPROVE_TOKEN: ${{ secrets.OPENCODE_APPROVE_TOKEN }} - AGENT_DISPATCH_TOKEN: ${{ github.token }} run: | set -euo pipefail if [ -n "$PR_REVIEW_MERGE_TOKEN" ]; then @@ -172,6 +175,7 @@ jobs: TARGET_REPOSITORY_SOURCE="${TARGET_REPOSITORY_TOKEN:+installation}" fi export TARGET_REPOSITORY_TOKEN + export AGENT_DISPATCH_TOKEN="$TARGET_REPOSITORY_TOKEN" if [ -z "$TARGET_REPOSITORY_TOKEN" ] || [ -z "$TARGET_REPOSITORY_SOURCE" ]; then echo "::error::Agent mention sweep requires PR_REVIEW_MERGE_TOKEN, OPENCODE_APPROVE_TOKEN, or the OpenCode app token exchange." exit 1 diff --git a/docs/automation/review-agent-comment-invocation.md b/docs/automation/review-agent-comment-invocation.md index a886caa96..2282b9d37 100644 --- a/docs/automation/review-agent-comment-invocation.md +++ b/docs/automation/review-agent-comment-invocation.md @@ -45,7 +45,7 @@ This preserves the central MSA boundary without copying privileged workflow code - The two agent-specific wrapper workflows receive only job-scoped `actions: read` and `contents: write`; their workflow defaults remain `contents: read`. - `actions: read` permits exact-name artifact inventory checks. Artifact upload uses the workflow artifact service and is pinned to immutable `actions/upload-artifact` v7.0.1. - `contents: write` is intentionally retained only on jobs that call GitHub's create-repository-dispatch endpoint. GitHub documents that endpoint as requiring Contents repository permission at write level. Removing it would disable the bounded central dispatch path; broad workflow-default write access is not granted. -- The organization sweep uses the established cross-repository credential chain for reading target comments, while the central repository's own short-lived job token dispatches the central workflows. +- The organization sweep uses the established cross-repository credential chain for reading target comments and dispatching central workflows. The local path uses that established review credential chain only for privileged `repository_dispatch`; its job-scoped `GITHUB_TOKEN` remains limited to target-repository metadata and acknowledgement UX. - OpenCode dispatch is restricted to the exact `OPENCODE_REPOSITORY_DISPATCH_TARGETS` allowlist. - An invocation cannot merge: `enable_auto_merge=false`, `update_branches=false`, and `merge_mode=disabled` are bound into the OpenCode invocation claim and hardcoded in the wrapper. GitHub's create-repository-dispatch endpoint allows at most 10 top-level `client_payload` properties (HTTP 422 otherwise), so those review-only constants are not copied onto the first-hop mention payload. The wrapper's merge-scheduler forward keeps those three flags and explicitly sends `trigger_reviews=true`, together with repository, PR, head/base SHA, base branch, and the invocation key. The source comment remains bound and auditable in the verified invocation claim and durable ledger; it is not repeated to the scheduler, which does not consume it. - Every dispatch is bound to live PR number, current head SHA, base branch, source comment, requested agent, and requesting actor metadata fetched or validated immediately before dispatch. diff --git a/scripts/ci/agent_mention_router.py b/scripts/ci/agent_mention_router.py index ee9232ebd..04725d1b2 100755 --- a/scripts/ci/agent_mention_router.py +++ b/scripts/ci/agent_mention_router.py @@ -10,8 +10,9 @@ import re import subprocess import time +from collections.abc import Sequence from dataclasses import dataclass -from typing import Any, Sequence +from typing import Any CENTRAL_AUTOMATION_REPOSITORY = "ContextualWisdomLab/.github" TRUSTED_ASSOCIATIONS = frozenset({"OWNER", "MEMBER", "COLLABORATOR"}) @@ -336,20 +337,26 @@ def _artifact_records( """ if not isinstance(value, dict): - raise ValueError("artifact response must be an object") + raise ValueError( # noqa: TRY004 - malformed external data is validation failure + "artifact response must be an object" + ) total_count = value.get("total_count") artifacts = value.get("artifacts") if type(total_count) is not int or total_count < 0: raise ValueError("artifact response has an invalid total_count") if not isinstance(artifacts, list): - raise ValueError("artifact response has an invalid artifacts collection") + raise ValueError( # noqa: TRY004 - malformed external data is validation failure + "artifact response has an invalid artifacts collection" + ) if total_count != len(artifacts): raise ValueError("artifact response is truncated or internally inconsistent") live: list[dict[str, Any]] = [] for artifact in artifacts: if not isinstance(artifact, dict): - raise ValueError("artifact response contains a non-object record") + raise ValueError( # noqa: TRY004 - malformed external data is validation failure + "artifact response contains a non-object record" + ) artifact_id = artifact.get("id") name = artifact.get("name") expired = artifact.get("expired") @@ -611,7 +618,9 @@ def load_event(path: str) -> dict[str, Any]: with open(path, encoding="utf-8") as handle: value = json.load(handle) if not isinstance(value, dict): - raise ValueError("GitHub event payload must be a JSON object") + raise ValueError( # noqa: TRY004 - malformed external data is validation failure + "GitHub event payload must be a JSON object" + ) return value diff --git a/tests/test_agent_mention_downstream_idempotency.py b/tests/test_agent_mention_downstream_idempotency.py index 23634f293..4062798f6 100644 --- a/tests/test_agent_mention_downstream_idempotency.py +++ b/tests/test_agent_mention_downstream_idempotency.py @@ -18,8 +18,11 @@ def test_router_can_read_durable_central_artifacts() -> None: local, sweep = text.split("\n sweep-organization-agent-mentions:\n", 1) assert "permissions:\n actions: read" in local assert "permissions:\n actions: read" in sweep - assert "AGENT_DISPATCH_TOKEN: ${{ github.token }}" in local - assert "AGENT_DISPATCH_TOKEN: ${{ github.token }}" in sweep + assert ( + "AGENT_DISPATCH_TOKEN: ${{ secrets.PR_REVIEW_MERGE_TOKEN || secrets.OPENCODE_APPROVE_TOKEN }}" + in local + ) + assert 'export AGENT_DISPATCH_TOKEN="$TARGET_REPOSITORY_TOKEN"' in sweep def test_downstream_workflows_claim_artifacts_and_bind_exact_key() -> None: diff --git a/tests/test_agent_mention_idempotency.py b/tests/test_agent_mention_idempotency.py index fc112116d..262f2fc27 100644 --- a/tests/test_agent_mention_idempotency.py +++ b/tests/test_agent_mention_idempotency.py @@ -348,3 +348,23 @@ def test_reaction_or_ack_failure_cannot_redispatch_completed_agents() -> None: ) == () assert dispatch_events(retry) == [] assert len(retry_target.calls) == 2 + + +def test_acknowledgement_comment_failure_is_auditable_without_failing_dispatch() -> None: + """A target comment permission failure preserves durable dispatch and retryability.""" + + module = load_module() + mention_request = request(module) + central = ArtifactAwareClient() + failing_target = ArtifactAwareClient(fail_target_call=2) + + 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", + ] diff --git a/tests/test_agent_mention_queue_isolation.py b/tests/test_agent_mention_queue_isolation.py index 8af11e04a..33a46e44f 100644 --- a/tests/test_agent_mention_queue_isolation.py +++ b/tests/test_agent_mention_queue_isolation.py @@ -46,8 +46,8 @@ def test_interactive_mentions_and_sweeps_use_independent_queues() -> None: assert not any(line.startswith("concurrency:") for line in header.splitlines()) assert _concurrency_block(local_job) == ( " concurrency:\n" - " group: review-agent-mention-router-local-${{ github.repository }}\n" - " queue: max" + " group: review-agent-mention-router-local-${{ github.repository }}-${{ github.event.comment.id }}\n" + " cancel-in-progress: false" ) assert _concurrency_block(sweep_job) == ( " concurrency:\n" @@ -67,5 +67,5 @@ def test_interactive_queue_retains_pending_requests_without_cancellation() -> No ) concurrency = _concurrency_block(local_job) - assert "queue: max" in concurrency - assert "cancel-in-progress: true" not in concurrency + assert "github.event.comment.id" in concurrency + assert "cancel-in-progress: false" in concurrency diff --git a/tests/test_agent_mention_workflow_contract.py b/tests/test_agent_mention_workflow_contract.py index c5fc4cae5..6b7cda0a6 100644 --- a/tests/test_agent_mention_workflow_contract.py +++ b/tests/test_agent_mention_workflow_contract.py @@ -37,6 +37,11 @@ def test_workflow_uses_local_event_and_central_sweep_with_job_scoped_writes() -> assert f" {permission}" in local assert "ref: ${{ github.event.repository.default_branch }}" in local assert "TARGET_REPOSITORY_TOKEN: ${{ github.token }}" in local + assert ( + "AGENT_DISPATCH_TOKEN: ${{ secrets.PR_REVIEW_MERGE_TOKEN || secrets.OPENCODE_APPROVE_TOKEN }}" + in local + ) + assert "the mention remains available for the scheduled sweep" in local assert "conversation_comments" not in local for permission in ("actions: read", "contents: write", "id-token: write"): @@ -47,7 +52,8 @@ def test_workflow_uses_local_event_and_central_sweep_with_job_scoped_writes() -> assert "secrets.PR_REVIEW_MERGE_TOKEN" in sweep assert "secrets.OPENCODE_APPROVE_TOKEN" in sweep assert "TARGET_REPOSITORY_SOURCE" in sweep - assert "AGENT_DISPATCH_TOKEN: ${{ github.token }}" in sweep + assert 'export AGENT_DISPATCH_TOKEN="$TARGET_REPOSITORY_TOKEN"' in sweep + assert "AGENT_DISPATCH_TOKEN: ${{ github.token }}" not in sweep assert "agent_mention_sweep.py" in sweep