Skip to content
Closed
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/agent-mention-router.yml
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ jobs:
&& (
contains(github.event.comment.body, '@cwl-noema-review')
|| contains(github.event.comment.body, '@opencode-agent')
|| contains(github.event.comment.body, '/opencode')
|| contains(github.event.comment.body, '/oc')
Comment thread
seonghobae marked this conversation as resolved.
)
concurrency:
group: review-agent-mention-router-local-${{ github.repository }}
Expand Down
14 changes: 14 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,20 @@ this file. The format follows Keep a Changelog, and versioned releases follow
Semantic Versioning where the repository publishes a release.

## [Unreleased]
- Accept upstream OpenCode's own `/opencode` and `/oc` trigger phrases as aliases of the local
`@opencode-agent` mention in `agent_mention_router.py`'s `MENTION_PATTERNS` (and, transitively,
`agent_mention_sweep.py`'s scheduled organization sweep, which imports the same matcher), plus the
`agent-mention-router.yml` pre-filter. A commenter following OpenCode's public GitHub Action docs
(which document `/opencode`/`/oc`, not this org's locally-invented `@opencode-agent` mention) now
successfully dispatches the same request instead of silently triggering nothing.
Comment thread
seonghobae marked this conversation as resolved.
- Fix the `@cwl-noema-review/@opencode-agent` bare-slash separator alternative in
`agent_mention_router.py`'s `MENTION_PATTERNS`: it previously checked only that the slash
immediately preceding `/@opencode-agent` was preceded by the literal text `@cwl-noema-review`,
without checking that occurrence's own left boundary, so invalid pasted text embedding the Noema
mention in a larger token (`foo@cwl-noema-review/@opencode-agent`,
`docs/@cwl-noema-review/@opencode-agent`) still dispatched an unintended OpenCode review. The
alternative now matches the whole `@cwl-noema-review/@opencode-agent` literal under the same
left-boundary exclusion as the standalone `@opencode-agent` alternative.
- Fail closed when the first top-level Noema JSON candidate is malformed,
preventing a later approval object from overriding malformed preface data;
multiple-object output remains supported when its first object is valid.
Expand Down
4 changes: 2 additions & 2 deletions docs/automation/review-agent-comment-invocation.md
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
# Review-agent comment invocation

Updated: 2026-08-22
Updated: 2026-09-01

## Purpose

Trusted ContextualWisdomLab maintainers can invoke the existing review planes from a pull-request conversation:

- `@cwl-noema-review` requests the independent Noema review.
- `@opencode-agent` requests a bounded current-head OpenCode review only; the invocation itself disables branch updates, automatic merge, and direct merge.
- `@opencode-agent` (or upstream OpenCode's own `/opencode`/`/oc` comment triggers, accepted as aliases of the same request) requests a bounded current-head OpenCode review only; the invocation itself disables branch updates, automatic merge, and direct merge.

The router never checks out or executes pull-request-controlled code. It reads live PR metadata, binds the request to the current head SHA and base branch, and dispatches the already deployed central workflows in `ContextualWisdomLab/.github`.

Expand Down
28 changes: 27 additions & 1 deletion scripts/ci/agent_mention_router.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,39 @@

CENTRAL_AUTOMATION_REPOSITORY = "ContextualWisdomLab/.github"
TRUSTED_ASSOCIATIONS = frozenset({"OWNER", "MEMBER", "COLLABORATOR"})
# "opencode-agent" also accepts /opencode and /oc: upstream OpenCode's own
# GitHub Action documents those as its trigger phrases
# (https://open-code.ai/en/docs/github), and this repo's dispatch pipeline
# accepts them as aliases of the same @opencode-agent request rather than
# forcing commenters to learn a locally-invented mention instead.
#
# None of the three alternatives below may be preceded by a bare "/": a
# preceding slash almost always means the match is embedded in a URL path
# (e.g. https://opencode.ai/docs, https://youtube.com/@opencode-agent) or an
# ordinary path segment (docs/@opencode-agent), not a deliberate trigger. The
# one deliberate exception is a maintainer separating both supported agent
# requests with a bare slash and no space (@cwl-noema-review/@opencode-agent).
# That case is matched as one combined literal — "@cwl-noema-review/@opencode-agent"
# — guarded by the same left-boundary exclusion as the standalone
# "@opencode-agent" alternative. A boundary check on the trailing slash alone
# is not enough: it would still fire for invalid pasted text where
# "@cwl-noema-review" is itself embedded in a larger token (e.g.
# foo@cwl-noema-review/@opencode-agent, docs/@cwl-noema-review/@opencode-agent)
# without checking that the Noema mention has a valid left boundary of its own.
# The bare /opencode and /oc forms additionally exclude a preceding "=": a
# URL query string (?next=/opencode, ?redirect=/oc) shares the same "not
# preceded by a word character" shape as a deliberate standalone command.
MENTION_PATTERNS = {
"cwl-noema-review": re.compile(
r"(?<![A-Za-z0-9_-])@cwl-noema-review(?![A-Za-z0-9_-])",
re.IGNORECASE,
),
"opencode-agent": re.compile(
r"(?<![A-Za-z0-9_-])@opencode-agent(?![A-Za-z0-9_-])",
r"(?:"
r"(?<![A-Za-z0-9_/-])@opencode-agent"
r"|(?<![A-Za-z0-9_/-])@cwl-noema-review/@opencode-agent"
r"|(?<![A-Za-z0-9_/=-])(?:/opencode|/oc)"
r")(?![A-Za-z0-9_-])",
re.IGNORECASE,
),
}
Expand Down
125 changes: 125 additions & 0 deletions tests/test_agent_mention_router.py
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,131 @@ def test_exact_mentions_and_parse_event() -> None:
assert module.exact_mentions("@opencode-agent-evil @cwl-noema-review2") == ()


@pytest.mark.parametrize(
"body",
[
"/opencode please re-review",
"/oc please re-review",
"kicking off /oc",
"/OC",
"/OpenCode",
],
)
def test_exact_mentions_accepts_slash_opencode_aliases(body: str) -> None:
"""Upstream OpenCode's own /opencode and /oc trigger phrases also dispatch."""

module = load_module()
assert module.exact_mentions(body) == ("opencode-agent",)


def test_exact_mentions_accepts_at_mention_after_a_slash_separator() -> None:
"""A slash used to separate two agent requests must not swallow the @mention.

Devin/owner review regression on #1537, across three rounds:

1. Excluding a preceding ``/`` from the lookbehind to reject
documentation-link false positives (see
``test_exact_mentions_rejects_slash_opencode_substrings``) was
originally applied to the whole ``@opencode-agent|/opencode|/oc``
alternation, so a maintainer separating both requested agents with a
bare slash and no space (``@cwl-noema-review/@opencode-agent``)
silently lost the OpenCode request.
2. Simply exempting the ``@`` form from the slash exclusion reopened the
same false-positive class for ``/@opencode-agent`` embedded in an
arbitrary URL or path segment.
3. Recognizing ``/@opencode-agent`` only when the slash is immediately
preceded by the other pattern's exact literal mention text
(``@cwl-noema-review``) checked only the boundary of the trailing
slash, not whether that ``@cwl-noema-review`` occurrence itself has a
valid left boundary, so invalid pasted text such as
``foo@cwl-noema-review/@opencode-agent`` still dispatched OpenCode
(see ``test_exact_mentions_rejects_invalid_separator_prefixes``).

The final pattern matches the whole separator form
(``@cwl-noema-review/@opencode-agent``) as one literal, guarded by the
same left-boundary exclusion as the standalone ``@opencode-agent``
alternative.
"""

module = load_module()
assert module.exact_mentions("@cwl-noema-review/@opencode-agent") == (
"cwl-noema-review",
"opencode-agent",
)


@pytest.mark.parametrize(
"body",
[
"foo@cwl-noema-review/@opencode-agent",
"docs/@cwl-noema-review/@opencode-agent",
"user.name@cwl-noema-review/@opencode-agent",
],
)
def test_exact_mentions_rejects_invalid_separator_prefixes(body: str) -> None:
"""The combined separator literal must not fire when embedded in a larger token.

Fifth-round finding on #1537, reported directly by the repository owner
(not a review bot): the separator alternative
``(?<=@cwl-noema-review)/@opencode-agent`` only checked the literal text
immediately before the slash, not whether that ``@cwl-noema-review``
occurrence itself has a valid left boundary. Pasted text embedding the
Noema mention inside a larger token — a preceding word
(``foo@cwl-noema-review/@opencode-agent``), a path segment
(``docs/@cwl-noema-review/@opencode-agent``), or an email-like local part
(``user.name@cwl-noema-review/@opencode-agent``) — still dispatched an
unintended OpenCode review. The fix matches the whole
``@cwl-noema-review/@opencode-agent`` literal with the same left-boundary
exclusion as the standalone ``@opencode-agent`` alternative, so it no
longer fires unless the combined mention itself starts at a valid
boundary. Some of these inputs still independently match the unrelated,
pre-existing ``cwl-noema-review`` pattern (e.g. a preceding ``/`` is not
excluded there); that pattern predates this PR and is out of scope for
this fix, so only the OpenCode dispatch is asserted here.
"""

module = load_module()
assert "opencode-agent" not in module.exact_mentions(body)


@pytest.mark.parametrize(
"body",
[
"the /occupied seat",
"visit /oceanography for more",
"see /opencode-docs for the guide",
"check out https://opencode.ai/docs for more info",
"see http://open-code.ai/en/docs/github",
"share this: https://youtube.com/@opencode-agent",
"see docs/@opencode-agent for the config file",
"visit https://example.com/?next=/opencode for the redirect",
"visit https://example.com/?next=/oc for the redirect",
],
)
def test_exact_mentions_rejects_slash_opencode_substrings(body: str) -> None:
"""A longer token merely starting with /oc or /opencode is not a mention.

Includes a URL whose path component happens to embed ``/opencode`` right
after the scheme's own ``//`` (Devin review finding on #1537): the prior
lookbehind excluded a preceding letter/digit/underscore/hyphen but not a
preceding ``/``, so a documentation link like ``https://opencode.ai``
satisfied it and could launch an unintended review. Also includes a
second-round Devin finding on the same PR: restoring plain recognition of
``@opencode-agent`` after a bare slash (so a maintainer could write
``@cwl-noema-review/@opencode-agent`` with no space) reopened the same
class of false positive for ``/@opencode-agent`` embedded in an arbitrary
URL or path segment, since both share the exact same "word char, then
slash, then the mention" shape as the deliberate separator case. A third
finding (CodeRabbit, same PR) noted the slash-preceded exclusion for the
bare ``/opencode``/``/oc`` forms did not also exclude a preceding ``=``,
so a URL query string such as ``?next=/opencode`` or ``?next=/oc`` still
matched.
"""

module = load_module()
assert module.exact_mentions(body) == ()


@pytest.mark.parametrize(
"payload",
[
Expand Down
7 changes: 7 additions & 0 deletions tests/test_pr_review_fix_scheduler.py
Original file line number Diff line number Diff line change
Expand Up @@ -1142,6 +1142,13 @@ def test_fix_inspect_skip_wait_and_error_paths(monkeypatch):
"""Inspect and queue logic report skip, wait, dispatch-limit, and errors."""
args = fix.parse_args(["--repo", "owner/repo", "--base-branch", "main"])
assert fix.inspect_pr("owner/repo", make_pr(isDraft=True), args) == ("skip", ("draft PR",))
assert fix.inspect_pr(
"owner/repo", make_pr(mergeStateStatus="DIRTY", isDraft=True), args
) == ("skip", ("draft PR",))
assert fix.inspect_pr("owner/repo", make_pr(mergeStateStatus="DIRTY"), args) == (
"skip",
("merge conflict is not authorized for repair",),
)
assert fix.inspect_pr("owner/repo", make_pr(baseRefName="develop"), args)[1][0].startswith("base branch")
wildcard_args = fix.parse_args(["--repo", "owner/repo", "--base-branch", "*"])
monkeypatch.setattr(fix, "needs_autofix", lambda pr: (False, ()))
Expand Down
48 changes: 48 additions & 0 deletions tests/test_pr_review_merge_scheduler.py
Original file line number Diff line number Diff line change
Expand Up @@ -717,6 +717,54 @@ def fake_api(path):
sched.fetch_all_pr_reviews_rest("owner/repo", 7)


def test_fetch_workflow_names_by_check_suite_rest_paginates_and_filters_incomplete_rows(monkeypatch):
"""The REST workflow-name lookup must paginate and skip unusable rows."""
page1_runs = [{"check_suite_id": i, "name": f"workflow-{i}"} for i in range(99)]
page1_runs.append({"check_suite_id": 99, "name": ""})
calls = []

def fake_api(path):
calls.append(path)
if path.endswith("page=1"):
return {"workflow_runs": page1_runs}
return {"workflow_runs": [{"check_suite_id": 100, "name": "opencode-review"}]}

monkeypatch.setattr(sched, "gh_api_json", fake_api)

names = sched.fetch_workflow_names_by_check_suite_rest("owner/repo", "a" * 40)

assert names[0] == "workflow-0"
assert 99 not in names
assert names[100] == "opencode-review"
assert calls == [
f"repos/owner/repo/actions/runs?head_sha={'a' * 40}&per_page=100&page=1",
f"repos/owner/repo/actions/runs?head_sha={'a' * 40}&per_page=100&page=2",
]


def test_fetch_workflow_names_by_check_suite_rest_returns_empty_map_when_resource_inaccessible(monkeypatch):
"""A denied Actions read must fail closed to an empty map, not raise."""

def fake_api(path):
raise RuntimeError("Resource not accessible by integration")

monkeypatch.setattr(sched, "gh_api_json", fake_api)

assert sched.fetch_workflow_names_by_check_suite_rest("owner/repo", "a" * 40) == {}


def test_fetch_workflow_names_by_check_suite_rest_propagates_other_failures(monkeypatch):
"""A transient REST failure unrelated to permissions must not be swallowed."""

def fake_api(path):
raise RuntimeError("gh: HTTP 502 (exhausted retries)")

monkeypatch.setattr(sched, "gh_api_json", fake_api)

with pytest.raises(RuntimeError, match="HTTP 502"):
sched.fetch_workflow_names_by_check_suite_rest("owner/repo", "a" * 40)


def test_fetch_pr_pagination_recovers_independent_approval_past_100_reviews(monkeypatch):
"""End-to-end regression for the reported bug: a genuine independent
APPROVED review made early in a PR's life must still satisfy
Expand Down
Loading