From 6b46f40b8df504a3f78513765a1bef7f75281183 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 1 Sep 2026 01:08:41 +0000 Subject: [PATCH 1/7] fix(agent-mention): accept upstream /opencode and /oc trigger aliases Owner-flagged gap: OpenCode's own GitHub Action documents /opencode and /oc as its trigger phrases, but this repo's agent_mention_router.py only recognized the locally-invented @opencode-agent mention. A commenter following OpenCode's public docs would silently trigger nothing. Widen the existing "opencode-agent" MENTION_PATTERNS regex to accept /opencode and /oc as aliases of the same request (agent_mention_sweep.py picks this up transitively since it imports the same matcher), and add the same substrings to agent-mention-router.yml's cheap pre-filter. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01KPmJErfkcHer4UVEgrQxUX --- .github/workflows/agent-mention-router.yml | 2 ++ CHANGELOG.md | 6 ++++ scripts/ci/agent_mention_router.py | 7 ++++- tests/test_agent_mention_router.py | 32 ++++++++++++++++++++++ 4 files changed, 46 insertions(+), 1 deletion(-) diff --git a/.github/workflows/agent-mention-router.yml b/.github/workflows/agent-mention-router.yml index 43fb163975..a109c8a97c 100644 --- a/.github/workflows/agent-mention-router.yml +++ b/.github/workflows/agent-mention-router.yml @@ -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') ) concurrency: group: review-agent-mention-router-local-${{ github.repository }} diff --git a/CHANGELOG.md b/CHANGELOG.md index 39c61c142b..adda248d33 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,12 @@ 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. - Harden the review sidecar's per-account catalog cap against silent drift: `contextual_orchestrator_review_launcher.py`'s two `build_zdr_prioritized_catalog` call sites now source their diff --git a/scripts/ci/agent_mention_router.py b/scripts/ci/agent_mention_router.py index ee9232ebd5..8471c13a2c 100755 --- a/scripts/ci/agent_mention_router.py +++ b/scripts/ci/agent_mention_router.py @@ -15,13 +15,18 @@ 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. MENTION_PATTERNS = { "cwl-noema-review": re.compile( r"(? 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",) + + +@pytest.mark.parametrize( + "body", + [ + "the /occupied seat", + "visit /oceanography for more", + "see /opencode-docs for the guide", + ], +) +def test_exact_mentions_rejects_slash_opencode_substrings(body: str) -> None: + """A longer token merely starting with /oc or /opencode is not a mention.""" + + module = load_module() + assert module.exact_mentions(body) == () + + @pytest.mark.parametrize( "payload", [ From 5318af6745d4dc1145cbcc204ce1ce3a4201374c Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 1 Sep 2026 05:09:00 +0000 Subject: [PATCH 2/7] test(scheduler): close #1541's 99% coverage regression on main main currently sits at 99% coverage (fail_under = 100), not caused by this branch: pr_review_fix_scheduler.py's inspect_pr() has two unexercised skip paths for a conflicted PR (draft, and not-authorized-for-repair), and pr_review_merge_scheduler.py's fetch_workflow_names_by_check_suite_rest() (added by #1541, never directly tested) has no coverage for a non- permission RuntimeError propagating, a row with a check_suite_id but no workflow name being skipped, or pagination continuing past a full page. Verified the same 99% gap reproduces on origin/main alone before writing these tests. Carried onto this branch so #1537 can pass its own 100% coverage gate without waiting on a separate fix PR to merge first; also opening that fix directly against main. --- tests/test_pr_review_fix_scheduler.py | 7 ++++ tests/test_pr_review_merge_scheduler.py | 48 +++++++++++++++++++++++++ 2 files changed, 55 insertions(+) diff --git a/tests/test_pr_review_fix_scheduler.py b/tests/test_pr_review_fix_scheduler.py index 3b4416bdc3..90a0169f86 100644 --- a/tests/test_pr_review_fix_scheduler.py +++ b/tests/test_pr_review_fix_scheduler.py @@ -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, ())) diff --git a/tests/test_pr_review_merge_scheduler.py b/tests/test_pr_review_merge_scheduler.py index 919566aeb2..a3e9547e63 100644 --- a/tests/test_pr_review_merge_scheduler.py +++ b/tests/test_pr_review_merge_scheduler.py @@ -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 From a059780fc499ecb9d4b79b689d8b6fe585459c24 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 1 Sep 2026 05:16:51 +0000 Subject: [PATCH 3/7] fix(agent-mention): exclude a preceding slash from the opencode alias match Devin review finding on #1537: the mention regex's negative lookbehind excluded a preceding letter/digit/underscore/hyphen but not a preceding slash, so a documentation link like https://opencode.ai/docs (where "/opencode" immediately follows the scheme's own "//") satisfied the lookbehind and could launch an unintended review. Added "/" to the excluded lookbehind character class, verified against the existing accept/reject cases plus the new URL-substring regression. Also updates docs/automation/review-agent-comment-invocation.md's operator guide (Devin's second finding) to list /opencode and /oc alongside @opencode-agent, matching the now-accepted trigger phrases. --- docs/automation/review-agent-comment-invocation.md | 4 ++-- scripts/ci/agent_mention_router.py | 2 +- tests/test_agent_mention_router.py | 11 ++++++++++- 3 files changed, 13 insertions(+), 4 deletions(-) diff --git a/docs/automation/review-agent-comment-invocation.md b/docs/automation/review-agent-comment-invocation.md index a886caa967..926249b563 100644 --- a/docs/automation/review-agent-comment-invocation.md +++ b/docs/automation/review-agent-comment-invocation.md @@ -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`. diff --git a/scripts/ci/agent_mention_router.py b/scripts/ci/agent_mention_router.py index 8471c13a2c..09dba62eb2 100755 --- a/scripts/ci/agent_mention_router.py +++ b/scripts/ci/agent_mention_router.py @@ -26,7 +26,7 @@ re.IGNORECASE, ), "opencode-agent": re.compile( - r"(? None: "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", ], ) def test_exact_mentions_rejects_slash_opencode_substrings(body: str) -> None: - """A longer token merely starting with /oc or /opencode is not a mention.""" + """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. + """ module = load_module() assert module.exact_mentions(body) == () From ed7cbdcacabe75e145ac5a0c560cf572c4d350b2 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 1 Sep 2026 05:21:58 +0000 Subject: [PATCH 4/7] fix(agent-mention): scope the slash exclusion to the bare slash aliases Devin review finding on #1537: the prior fix's slash-exclusion lookbehind applied to the whole @opencode-agent|/opencode|/oc alternation, so a maintainer separating two requested agents with a bare slash and no space (@cwl-noema-review/@opencode-agent) silently lost the OpenCode request -- the same lookbehind that correctly rejects a documentation-link false positive like https://opencode.ai now also rejected the intended @mention. Split the lookbehind per Devin's suggested fix: the slash exclusion applies only to the bare /opencode and /oc forms; @opencode-agent keeps its original lookbehind, unaffected by a preceding slash. --- scripts/ci/agent_mention_router.py | 3 ++- tests/test_agent_mention_router.py | 20 ++++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/scripts/ci/agent_mention_router.py b/scripts/ci/agent_mention_router.py index 09dba62eb2..1eab52cf04 100755 --- a/scripts/ci/agent_mention_router.py +++ b/scripts/ci/agent_mention_router.py @@ -26,7 +26,8 @@ re.IGNORECASE, ), "opencode-agent": re.compile( - r"(? None: 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 review regression on #1537: 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. The slash exclusion must apply only to the bare ``/opencode`` + and ``/oc`` forms, not to the ``@`` form. + """ + + module = load_module() + assert module.exact_mentions("@cwl-noema-review/@opencode-agent") == ( + "cwl-noema-review", + "opencode-agent", + ) + + @pytest.mark.parametrize( "body", [ From c62f7ebb71068a9dd3d5bd702d7cfee8064313b6 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 1 Sep 2026 05:30:54 +0000 Subject: [PATCH 5/7] fix(agent-mention): recognize /@opencode-agent only after the exact separator Devin review finding on #1537 (second round): the previous fix exempted the @opencode-agent form entirely from the slash-preceded exclusion so that @cwl-noema-review/@opencode-agent (a bare-slash separator) would still dispatch OpenCode. That reopened the exact false-positive class the first fix closed, just for the @ form instead of the /opencode //oc forms: a trusted comment sharing a link like https://youtube.com/@opencode-agent, or even an ordinary path segment like docs/@opencode-agent, would satisfy the same shape and launch an unintended review. Replaced the blanket exemption with a narrow third alternative: /@opencode-agent is recognized only when the slash is immediately preceded by the other pattern's exact literal mention text, @cwl-noema-review, not by an arbitrary preceding word. This keeps the deliberate separator case working while URL paths and ordinary text no longer qualify. --- scripts/ci/agent_mention_router.py | 16 +++++++++++++-- tests/test_agent_mention_router.py | 31 +++++++++++++++++++++--------- 2 files changed, 36 insertions(+), 11 deletions(-) diff --git a/scripts/ci/agent_mention_router.py b/scripts/ci/agent_mention_router.py index 1eab52cf04..504d6220c8 100755 --- a/scripts/ci/agent_mention_router.py +++ b/scripts/ci/agent_mention_router.py @@ -20,14 +20,26 @@ # (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 is recognized only when the slash is immediately preceded by the +# other pattern's exact literal mention text, not by an arbitrary word. MENTION_PATTERNS = { "cwl-noema-review": re.compile( r"(? None: 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 review regression on #1537: 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. The slash exclusion must apply only to the bare ``/opencode`` - and ``/oc`` forms, not to the ``@`` form. + Devin review regression on #1537, in two rounds: 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. 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 (a + second Devin finding). The final pattern instead recognizes + ``/@opencode-agent`` only when the slash is immediately preceded by the + other pattern's exact literal mention text (``@cwl-noema-review``), not + by an arbitrary preceding word. """ module = load_module() @@ -155,6 +160,8 @@ def test_exact_mentions_accepts_at_mention_after_a_slash_separator() -> None: "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", ], ) def test_exact_mentions_rejects_slash_opencode_substrings(body: str) -> None: @@ -164,7 +171,13 @@ def test_exact_mentions_rejects_slash_opencode_substrings(body: str) -> None: 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. + 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. """ module = load_module() From af3c7b879e4beb19bfe560ef8b29c4ba83f677a6 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 1 Sep 2026 05:34:09 +0000 Subject: [PATCH 6/7] fix(agent-mention): exclude a preceding = from the bare slash aliases CodeRabbit review finding on #1537: the bare /opencode and /oc forms' slash-exclusion lookbehind did not also exclude a preceding "=", so a URL query string such as https://example.com/?next=/opencode or ?next=/oc still matched and could dispatch an unintended review. Added "=" to the excluded lookbehind character class alongside the existing letter/digit/ underscore/hyphen/slash exclusions. --- scripts/ci/agent_mention_router.py | 5 ++++- tests/test_agent_mention_router.py | 8 +++++++- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/scripts/ci/agent_mention_router.py b/scripts/ci/agent_mention_router.py index 504d6220c8..a3b2255fb5 100755 --- a/scripts/ci/agent_mention_router.py +++ b/scripts/ci/agent_mention_router.py @@ -29,6 +29,9 @@ # requests with a bare slash and no space (@cwl-noema-review/@opencode-agent); # that is recognized only when the slash is immediately preceded by the # other pattern's exact literal mention text, not by an arbitrary word. +# 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"(? None: "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: @@ -177,7 +179,11 @@ def test_exact_mentions_rejects_slash_opencode_substrings(body: str) -> None: ``@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. + 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() From 1b36c5b0dcdb0e70cc3e7aa17aeed7497b3aa720 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 1 Sep 2026 05:44:51 +0000 Subject: [PATCH 7/7] fix(agent-mention): require a valid left boundary on the whole separator literal The @cwl-noema-review/@opencode-agent bare-slash separator alternative 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. Invalid pasted text embedding the Noema mention in a larger token (foo@cwl-noema-review/@opencode-agent, docs/@cwl-noema-review/@opencode-agent, an email-like local part) still dispatched an unintended OpenCode review. Reported directly by the repository owner with exact reproduction and a suggested direction; verified the owner's literal suggested regex still failed one of their own listed cases (docs/@cwl-noema-review/@opencode-agent) because it omitted "/" from the excluded lookbehind class. Fixed by matching the whole @cwl-noema-review/@opencode-agent literal under the same left-boundary exclusion already used by the standalone @opencode-agent alternative, verified against the full existing accept/reject matrix plus the three new cases before touching the source file. Full suite: 2246 passed, 1 skipped, 21 subtests passed. 100% coverage and 100% docstrings maintained. --- CHANGELOG.md | 8 ++++ scripts/ci/agent_mention_router.py | 13 ++++-- tests/test_agent_mention_router.py | 71 ++++++++++++++++++++++++------ 3 files changed, 75 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 74fda1e6cd..67f3f26f1f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,14 @@ Semantic Versioning where the repository publishes a release. `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. +- 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. diff --git a/scripts/ci/agent_mention_router.py b/scripts/ci/agent_mention_router.py index a3b2255fb5..b67b8a9212 100755 --- a/scripts/ci/agent_mention_router.py +++ b/scripts/ci/agent_mention_router.py @@ -26,9 +26,14 @@ # (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 is recognized only when the slash is immediately preceded by the -# other pattern's exact literal mention text, not by an arbitrary word. +# 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. @@ -40,7 +45,7 @@ "opencode-agent": re.compile( r"(?:" r"(? None: 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 review regression on #1537, in two rounds: 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. 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 (a - second Devin finding). The final pattern instead recognizes - ``/@opencode-agent`` only when the slash is immediately preceded by the - other pattern's exact literal mention text (``@cwl-noema-review``), not - by an arbitrary preceding word. + 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() @@ -152,6 +163,40 @@ def test_exact_mentions_accepts_at_mention_after_a_slash_separator() -> None: ) +@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", [