Skip to content

Commit 19ced89

Browse files
seonghobaeseonghobae
committed
fix(ci): bound review dispatch payloads and UX failures
1 parent 67d834f commit 19ced89

8 files changed

Lines changed: 126 additions & 38 deletions

‎.github/workflows/agent-mention-noema-dispatch.yml‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ on:
1111
concurrency:
1212
group: agent-mention-noema-${{ github.event.client_payload.agent_invocation_key || github.run_id }}
1313
cancel-in-progress: false
14-
queue: max
1514

1615
permissions:
1716
contents: read

‎.github/workflows/agent-mention-opencode-dispatch.yml‎

Lines changed: 6 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ on:
1111
concurrency:
1212
group: agent-mention-opencode-${{ github.event.client_payload.agent_invocation_key || github.run_id }}
1313
cancel-in-progress: false
14-
queue: max
1514

1615
permissions:
1716
contents: read
@@ -36,11 +35,11 @@ jobs:
3635
BASE_BRANCH: ${{ github.event.client_payload.base_branch || '' }}
3736
REQUESTED_BY: ${{ github.event.client_payload.requested_by || '' }}
3837
SOURCE_COMMENT_ID: ${{ github.event.client_payload.source_comment_id || '' }}
39-
TRIGGER_REVIEWS: ${{ github.event.client_payload.trigger_reviews }}
40-
REVIEW_DISPATCH_LIMIT: ${{ github.event.client_payload.review_dispatch_limit || '' }}
41-
ENABLE_AUTO_MERGE: ${{ github.event.client_payload.enable_auto_merge }}
42-
UPDATE_BRANCHES: ${{ github.event.client_payload.update_branches }}
43-
MERGE_MODE: ${{ github.event.client_payload.merge_mode || '' }}
38+
TRIGGER_REVIEWS: ${{ github.event.client_payload.review_policy.trigger_reviews }}
39+
REVIEW_DISPATCH_LIMIT: ${{ github.event.client_payload.review_policy.review_dispatch_limit || '' }}
40+
ENABLE_AUTO_MERGE: ${{ github.event.client_payload.review_policy.enable_auto_merge }}
41+
UPDATE_BRANCHES: ${{ github.event.client_payload.review_policy.update_branches }}
42+
MERGE_MODE: ${{ github.event.client_payload.review_policy.merge_mode || '' }}
4443
steps:
4544
- name: Validate exact invocation payload
4645
run: |
@@ -195,10 +194,6 @@ jobs:
195194
--arg pr_head_sha "$PR_HEAD_SHA" \
196195
--arg pr_base_sha "$PR_BASE_SHA" \
197196
--arg base_branch "$BASE_BRANCH" \
198-
--arg requested_agent "$REQUESTED_AGENT" \
199-
--arg agent_invocation_key "$INVOCATION_KEY" \
200-
--arg requested_by "$REQUESTED_BY" \
201-
--argjson source_comment_id "$SOURCE_COMMENT_ID" \
202197
'{
203198
event_type: "merge-scheduler",
204199
client_payload: {
@@ -211,11 +206,7 @@ jobs:
211206
review_dispatch_limit: "1",
212207
enable_auto_merge: false,
213208
update_branches: false,
214-
merge_mode: "disabled",
215-
requested_agent: $requested_agent,
216-
agent_invocation_key: $agent_invocation_key,
217-
requested_by: $requested_by,
218-
source_comment_id: $source_comment_id
209+
merge_mode: "disabled"
219210
}
220211
}' \
221212
| gh api "repos/${GITHUB_REPOSITORY}/dispatches" -X POST --input -

‎docs/adr/0001-agent-mention-dispatch-contract.md‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
# ADR-0001: Bounded review-agent dispatch and non-authoritative acknowledgements
2+
3+
Status: **Proposed**
4+
Date: 2026-08-12
5+
6+
## Context
7+
8+
The trusted review-agent mention router was exercised against current PR heads
9+
on 2026-08-12. GitHub rejected the OpenCode `repository_dispatch` body because
10+
its `client_payload` contained 14 top-level properties while the API permits at
11+
most 10. A second run reached the dispatch path but failed while creating a
12+
target-repository reaction/acknowledgement with HTTP 403. Treating that UX
13+
failure as a dispatch failure could cause a later sweep to dispatch the same
14+
review again even though the durable central artifact claim already exists.
15+
16+
## Decision
17+
18+
1. Keep the complete exact-head/base/actor/comment/invocation-key claim, but
19+
place the five fixed review-only policy values under one `review_policy`
20+
object so every `repository_dispatch.client_payload` remains at or below
21+
GitHub's ten-property limit.
22+
2. The OpenCode policy remains fail-closed: review triggering is enabled,
23+
dispatch budget is one, auto-merge is disabled, branch updates are disabled,
24+
and merge mode is `disabled`. The nested policy is validated by the trusted
25+
wrapper before the artifact ledger or downstream scheduler is reached.
26+
3. Central dispatch and the durable artifact claim are authoritative. Target
27+
reactions and acknowledgement comments are best-effort UX signals; a
28+
permission or transport failure is logged without rethrowing after a
29+
successful central dispatch.
30+
4. Cross-repository sweeps must use the configured organization token or
31+
OpenCode installation token. A central `GITHUB_TOKEN` is not treated as a
32+
sibling-repository credential, and no review or merge authority is inferred
33+
from a router success.
34+
35+
## Consequences
36+
37+
The router can complete a review request when GitHub declines a nonessential
38+
reaction or acknowledgement, preventing duplicate dispatches. A missing
39+
acknowledgement remains visible in logs and does not become approval evidence.
40+
Nested policy decoding adds one explicit workflow boundary, covered by payload
41+
cardinality and exact-claim tests.
42+
43+
The dispatch wrappers also use only GitHub-supported concurrency keys. An
44+
unsupported `queue: max` setting was removed after actionlint rejected it; the
45+
non-cancelling invocation group remains the supported duplicate-control
46+
mechanism.
47+
48+
## Verification
49+
50+
- `tests/test_agent_mention_router.py` verifies the nested policy and ten-field
51+
limit.
52+
- `tests/test_agent_mention_idempotency.py` verifies target UX failures do not
53+
redispatch completed agents.
54+
- `tests/test_agent_mention_complete_payload_binding.py` verifies the nested
55+
policy preserves the exact invocation contract.
56+
- The wrapper workflow reads only `client_payload.review_policy` and forwards a
57+
ten-property scheduler payload.
58+
- `actionlint` accepts both dispatch wrapper workflows.
59+
- Independent current-head review, terminal checks, structured Strix evidence,
60+
and protected-branch rules remain required before merge.

‎scripts/ci/agent_mention_router.py‎

Lines changed: 33 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -402,11 +402,13 @@ def opencode_payload(request: MentionRequest) -> dict[str, Any]:
402402
"pr_head_sha": request.pull_request_head_sha,
403403
"pr_base_sha": request.pull_request_base_sha,
404404
"base_branch": request.pull_request_base_branch,
405-
"trigger_reviews": claim["trigger_reviews"],
406-
"review_dispatch_limit": claim["review_dispatch_limit"],
407-
"enable_auto_merge": claim["enable_auto_merge"],
408-
"update_branches": claim["update_branches"],
409-
"merge_mode": claim["merge_mode"],
405+
"review_policy": {
406+
"trigger_reviews": claim["trigger_reviews"],
407+
"review_dispatch_limit": claim["review_dispatch_limit"],
408+
"enable_auto_merge": claim["enable_auto_merge"],
409+
"update_branches": claim["update_branches"],
410+
"merge_mode": claim["merge_mode"],
411+
},
410412
"requested_agent": agent,
411413
"agent_invocation_key": agent_invocation_key(request, agent),
412414
"requested_by": request.actor,
@@ -415,6 +417,24 @@ def opencode_payload(request: MentionRequest) -> dict[str, Any]:
415417
}
416418

417419

420+
def _best_effort_target_request(
421+
target_client: GitHubClient,
422+
args: Sequence[str],
423+
input_payload: dict[str, Any],
424+
*,
425+
operation: str,
426+
) -> None:
427+
"""Keep target-repository UX failures from causing a duplicate dispatch."""
428+
429+
try:
430+
target_client.request(args, input_payload=input_payload)
431+
except RuntimeError as exc:
432+
print(
433+
"Target-repository "
434+
f"{operation} unavailable; dispatch remains authoritative: {str(exc)[:500]}"
435+
)
436+
437+
418438
def dispatch_request(
419439
request: MentionRequest,
420440
*,
@@ -478,13 +498,15 @@ def dispatch_request(
478498
ledger_artifact_cache[agent_ledger_artifact_name(request, agent)] = True
479499

480500
target_api = f"repos/{request.repository}"
481-
target_client.request(
501+
_best_effort_target_request(
502+
target_client,
482503
[
483504
f"{target_api}/issues/comments/{request.comment_id}/reactions",
484505
"-X",
485506
"POST",
486507
],
487-
input_payload={"content": "eyes"},
508+
{"content": "eyes"},
509+
operation="reaction",
488510
)
489511
status_parts = [f"Queued {' and '.join(handles)}"]
490512
existing_handles = tuple(
@@ -507,13 +529,15 @@ def dispatch_request(
507529
"are the durable dispatch ledger; existing review workflows remain "
508530
"authoritative for the final verdict and failure evidence."
509531
)
510-
target_client.request(
532+
_best_effort_target_request(
533+
target_client,
511534
[
512535
f"{target_api}/issues/{request.pull_request_number}/comments",
513536
"-X",
514537
"POST",
515538
],
516-
input_payload={"body": acknowledgement},
539+
{"body": acknowledgement},
540+
operation="acknowledgement",
517541
)
518542
return handles
519543

‎tests/test_agent_mention_complete_payload_binding.py‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,15 @@ def test_invocation_claim_binds_all_security_relevant_fields() -> None:
122122
"trigger_reviews": True,
123123
"update_branches": False,
124124
}
125+
opencode_payload = router.opencode_payload(request)["client_payload"]
126+
assert opencode_payload["review_policy"] == {
127+
"trigger_reviews": True,
128+
"review_dispatch_limit": "1",
129+
"enable_auto_merge": False,
130+
"update_branches": False,
131+
"merge_mode": "disabled",
132+
}
133+
assert len(opencode_payload) <= 10
125134

126135
noema_key = router.agent_invocation_key(request, "cwl-noema-review")
127136
opencode_key = router.agent_invocation_key(request, "opencode-agent")

‎tests/test_agent_mention_downstream_idempotency.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ def test_downstream_workflows_claim_artifacts_and_bind_exact_key() -> None:
3333
assert "source_comment_id" in text
3434
assert "requested_agent" in text
3535
assert "cancel-in-progress: false" in text
36-
assert "queue: max" in text
36+
assert "queue: max" not in text
3737
assert "^[0-9a-f]{64}$" in text
3838
assert "^[1-9][0-9]*$" in text
3939
assert "actions/artifacts" in text

‎tests/test_agent_mention_idempotency.py‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -320,13 +320,12 @@ def test_reaction_or_ack_failure_cannot_redispatch_completed_agents() -> None:
320320
mention_request = request(module)
321321
central = ArtifactAwareClient()
322322
failing_target = ArtifactAwareClient(fail_target_call=1)
323-
with pytest.raises(RuntimeError, match="target call"):
324-
module.dispatch_request(
325-
mention_request,
326-
target_client=failing_target,
327-
dispatch_client=central,
328-
opencode_allowlist=frozenset({mention_request.repository}),
329-
)
323+
assert module.dispatch_request(
324+
mention_request,
325+
target_client=failing_target,
326+
dispatch_client=central,
327+
opencode_allowlist=frozenset({mention_request.repository}),
328+
) == ("@cwl-noema-review", "@opencode-agent")
330329
assert dispatch_events(central) == [
331330
"agent-mention-noema",
332331
"agent-mention-opencode",

‎tests/test_agent_mention_router.py‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -220,11 +220,17 @@ def test_eligible_agents_and_payloads() -> None:
220220
assert noema["client_payload"]["pr_base_sha"] == "b" * 40
221221
opencode = module.opencode_payload(request)
222222
assert opencode["event_type"] == "agent-mention-opencode"
223-
assert opencode["client_payload"]["base_branch"] == "develop"
224-
assert opencode["client_payload"]["pr_base_sha"] == "b" * 40
225-
assert opencode["client_payload"]["merge_mode"] == "disabled"
226-
assert opencode["client_payload"]["enable_auto_merge"] is False
227-
assert opencode["client_payload"]["update_branches"] is False
223+
payload = opencode["client_payload"]
224+
assert payload["base_branch"] == "develop"
225+
assert payload["pr_base_sha"] == "b" * 40
226+
assert payload["review_policy"] == {
227+
"trigger_reviews": True,
228+
"review_dispatch_limit": "1",
229+
"enable_auto_merge": False,
230+
"update_branches": False,
231+
"merge_mode": "disabled",
232+
}
233+
assert len(payload) <= 10
228234

229235

230236
def test_dispatch_uses_central_events_and_acknowledges() -> None:

0 commit comments

Comments
 (0)