From 2009bcb9ffcdb1ab4f91c772d698778576ee6f3d Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Tue, 1 Sep 2026 15:08:29 +0900 Subject: [PATCH 01/12] test(scheduler): close current-main RCA coverage gaps --- ...est_scheduler_1541_coverage_regressions.py | 102 ++++++++++++++++++ 1 file changed, 102 insertions(+) create mode 100644 tests/test_scheduler_1541_coverage_regressions.py diff --git a/tests/test_scheduler_1541_coverage_regressions.py b/tests/test_scheduler_1541_coverage_regressions.py new file mode 100644 index 0000000000..3d03ce56db --- /dev/null +++ b/tests/test_scheduler_1541_coverage_regressions.py @@ -0,0 +1,102 @@ +"""Regression coverage for the scheduler branches introduced by PR #1541.""" + +from __future__ import annotations + +import pytest + +from scripts.ci import pr_review_fix_scheduler as fix +from scripts.ci import pr_review_merge_scheduler as merge + + +def _pr(**overrides: object) -> dict[str, object]: + """Return a minimal same-repository pull-request fixture.""" + value: dict[str, object] = { + "number": 7, + "isDraft": False, + "baseRefName": "main", + "baseRefOid": "b" * 40, + "headRefName": "feature", + "headRefOid": "a" * 40, + "headRepository": {"nameWithOwner": "owner/repo"}, + "mergeStateStatus": "CLEAN", + "reviews": {"nodes": []}, + "reviewThreads": {"nodes": []}, + } + value.update(overrides) + return value + + +def test_conflicted_draft_skips_before_repair_authority() -> None: + """A conflicted draft remains a draft skip rather than an RCA dispatch.""" + args = fix.parse_args(["--repo", "owner/repo", "--base-branch", "main"]) + + assert fix.inspect_pr( + "owner/repo", + _pr(isDraft=True, mergeStateStatus="DIRTY"), + args, + ) == ("skip", ("draft PR",)) + + +def test_conflicted_unapproved_pr_fails_closed_without_repair_authority() -> None: + """A conflict without explicit unreviewed-repair authority stays closed.""" + args = fix.parse_args(["--repo", "owner/repo", "--base-branch", "main"]) + + assert fix.inspect_pr( + "owner/repo", + _pr(mergeStateStatus="DIRTY"), + args, + ) == ("skip", ("merge conflict is not authorized for repair",)) + + +def test_workflow_name_rest_fallback_paginates_and_filters_rows( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Workflow identity pagination keeps only rows with usable names.""" + page_one = [{"check_suite_id": index, "name": f"workflow-{index}"} for index in range(99)] + page_one.append({"check_suite_id": 99, "name": ""}) + calls: list[str] = [] + + def fake_api(path: str) -> dict[str, object]: + calls.append(path) + if path.endswith("page=1"): + return {"workflow_runs": page_one} + return {"workflow_runs": [{"check_suite_id": 100, "name": "opencode-review"}]} + + monkeypatch.setattr(merge, "gh_api_json", fake_api) + + names = merge.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_workflow_name_rest_fallback_treats_permission_denial_as_unknown( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """An inaccessible Actions inventory returns an empty fail-closed map.""" + + def fake_api(_path: str) -> dict[str, object]: + raise RuntimeError("Resource not accessible by integration") + + monkeypatch.setattr(merge, "gh_api_json", fake_api) + + assert merge.fetch_workflow_names_by_check_suite_rest("owner/repo", "a" * 40) == {} + + +def test_workflow_name_rest_fallback_propagates_unrelated_failures( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Transport failures other than permission denial remain visible.""" + + def fake_api(_path: str) -> dict[str, object]: + raise RuntimeError("gh: HTTP 502 (exhausted retries)") + + monkeypatch.setattr(merge, "gh_api_json", fake_api) + + with pytest.raises(RuntimeError, match="HTTP 502"): + merge.fetch_workflow_names_by_check_suite_rest("owner/repo", "a" * 40) From 563a766c5be935a8cdba7afc0cc9684af741e72e Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Tue, 1 Sep 2026 15:46:35 +0900 Subject: [PATCH 02/12] test(scheduler): close remaining current-main coverage gaps --- tests/test_pr_review_fix_scheduler.py | 86 +++++++++++++++++++++++++++ 1 file changed, 86 insertions(+) diff --git a/tests/test_pr_review_fix_scheduler.py b/tests/test_pr_review_fix_scheduler.py index 9860eeaec7..daeefae573 100644 --- a/tests/test_pr_review_fix_scheduler.py +++ b/tests/test_pr_review_fix_scheduler.py @@ -177,6 +177,92 @@ def test_prepare_autofix_slot_preserves_new_head_workers_after_head_advance(monk workflow_repository=fix.DEFAULT_AUTOFIX_REPOSITORY, dry_run=False, ) is None + + +def test_prepare_autofix_slot_returns_same_head_with_no_stale_workers(monkeypatch): + """No stale workers means the cancellation branch is never entered.""" + head = "a" * 40 + monkeypatch.setattr( + fix, + "run_json", + lambda _args: { + "workflow_runs": [ + { + "id": 1, + "status": "in_progress", + "display_title": f"PR Review Autofix owner/repo#7@{head}", + } + ] + }, + ) + monkeypatch.setattr( + fix, + "force_cancel_workflow_runs", + lambda *_args: pytest.fail("there is no stale worker to cancel"), + ) + monkeypatch.setattr( + fix, + "live_head_matches", + lambda *_args: pytest.fail( + "staleness is only ever checked when a stale worker exists" + ), + ) + + assert fix.prepare_autofix_slot( + "owner/repo", + make_pr(headRefOid=head), + workflow=fix.DEFAULT_AUTOFIX_WORKFLOW, + workflow_repository=fix.DEFAULT_AUTOFIX_REPOSITORY, + dry_run=False, + ) + + +def test_live_head_matches_compares_the_live_head_to_the_cached_snapshot(monkeypatch): + """The real head-matching implementation reads GitHub's current PR head. + + Every other test in this module monkeypatches ``live_head_matches`` away, + so its own body (the ``gh api`` read, the payload-shape guard, and the + case-insensitive comparison) was never exercised by the suite at all. + """ + pr = make_pr(headRefOid="a" * 40) + + monkeypatch.setattr(fix, "run_json", lambda _args: {"head": {"sha": "A" * 40}}) + assert fix.live_head_matches("owner/repo", pr) + + monkeypatch.setattr(fix, "run_json", lambda _args: {"head": {"sha": "b" * 40}}) + assert not fix.live_head_matches("owner/repo", pr) + + monkeypatch.setattr(fix, "run_json", lambda _args: {"head": {"sha": "short"}}) + assert not fix.live_head_matches("owner/repo", pr) + + monkeypatch.setattr(fix, "run_json", lambda _args: {"head": {"sha": None}}) + assert not fix.live_head_matches("owner/repo", pr) + + monkeypatch.setattr(fix, "run_json", lambda _args: {"head": "not-a-dict"}) + assert not fix.live_head_matches("owner/repo", pr) + + monkeypatch.setattr(fix, "run_json", lambda _args: "not-a-dict") + assert not fix.live_head_matches("owner/repo", pr) + + +def test_inspect_pr_reports_active_autofix_worker_without_dispatch(monkeypatch): + """An already-running current-head worker waits instead of re-dispatching.""" + args = fix.parse_args(["--repo", "owner/repo", "--base-branch", "main"]) + monkeypatch.setattr(fix, "needs_autofix", lambda _pr: (True, ("review",))) + monkeypatch.setattr(fix, "issue_comments", lambda _repo, _number: []) + monkeypatch.setattr(fix, "prepare_autofix_slot", lambda *_args, **_kwargs: True) + monkeypatch.setattr( + fix, + "dispatch_autofix", + lambda *_args, **_kwargs: pytest.fail("an active worker must not be redispatched"), + ) + + assert fix.inspect_pr("owner/repo", make_pr(), args) == ( + "wait", + ("current-head autofix run is already queued or running",), + ) + + def test_terminal_failed_check_triggers_rca_without_prior_opencode_review(): """Exact-head check evidence can start RCA without a circular review prerequisite.""" pr = make_pr( From 8b26dab2f8743a286f430a9cdb96a51fa8dd2305 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Tue, 1 Sep 2026 15:48:50 +0900 Subject: [PATCH 03/12] fix(agent-mention): stack safe OpenCode slash aliases --- .github/workflows/agent-mention-router.yml | 2 + .../review-agent-comment-invocation.md | 4 +- scripts/ci/agent_mention_router.py | 38 ++++- tests/test_agent_mention_router.py | 154 ++++++++++++++++++ 4 files changed, 195 insertions(+), 3 deletions(-) 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/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 ee9232ebd5..498b885c27 100755 --- a/scripts/ci/agent_mention_router.py +++ b/scripts/ci/agent_mention_router.py @@ -15,13 +15,49 @@ 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. +# +# The shared trailing boundary after any of the three alternatives also +# excludes a following "/": without it, a root-relative path continuation +# right after the alias (/oc/config, /opencode/docs, @opencode-agent/config, +# @cwl-noema-review/@opencode-agent/foo) still matched, since the alias text +# itself is a complete, valid match and nothing in the original trailing +# lookahead treated "/" as a word character. This mirrors the +# leading-boundary "/" exclusion already applied above and closes the same +# false-positive class from the trailing side, for every alternative rather +# than only the bare /opencode and /oc forms. 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",) + + +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( + "body", + [ + "/oc/config", + "/opencode/docs", + "@opencode-agent/config", + "@cwl-noema-review/@opencode-agent/foo", + ], +) +def test_exact_mentions_rejects_trailing_path_continuation(body: str) -> None: + """A root-relative path continuation right after the alias is not a mention. + + Sixth-round finding on #1537's successor PR (Devin): the shared trailing + boundary after all three ``opencode-agent`` alternatives excluded a + following letter, digit, underscore, or hyphen but not a following + ``/``, so a root-relative path glued directly onto the alias — ``/oc`` + followed by ``/config``, ``/opencode`` followed by ``/docs``, or even + ``@opencode-agent`` or the ``@cwl-noema-review/@opencode-agent`` + separator followed by ``/config`` or ``/foo`` — still matched as a + complete, valid mention, since nothing treated the alias text itself as + incomplete just because a slash continued right after it. The fix adds + ``/`` to the shared trailing exclusion, mirroring the leading-boundary + ``/`` exclusion already applied to each alternative from the other side. + """ + + module = load_module() + assert "opencode-agent" not in module.exact_mentions(body) + + @pytest.mark.parametrize( "payload", [ From cd6a98618f504b1c5ab46ace65c4cc831e062fa6 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 1 Sep 2026 06:53:19 +0000 Subject: [PATCH 04/12] fix(agent-mention): exclude a trailing query string from the opencode-agent boundary The shared trailing lookahead excluded a following letter, digit, underscore, hyphen, or slash, but not a following "?", so a query string glued directly onto the alias with no separator (/oc?mode=docs, /opencode?next=x) still matched as a complete mention. Reported by CodeRabbit on this feature's predecessor PR (#1558, now closed in favor of this clean stack on #1554). Reproduced first, then added "?" to the same shared trailing exclusion, verified against the full existing accept/reject matrix plus the new query-string cases before applying. Full suite: 2278 passed, 1 skipped, 21 subtests passed. 100% coverage and 100% docstrings maintained. --- scripts/ci/agent_mention_router.py | 8 ++++++-- tests/test_agent_mention_router.py | 24 ++++++++++++++++++++++++ 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/scripts/ci/agent_mention_router.py b/scripts/ci/agent_mention_router.py index 498b885c27..da2f257826 100755 --- a/scripts/ci/agent_mention_router.py +++ b/scripts/ci/agent_mention_router.py @@ -46,7 +46,11 @@ # lookahead treated "/" as a word character. This mirrors the # leading-boundary "/" exclusion already applied above and closes the same # false-positive class from the trailing side, for every alternative rather -# than only the bare /opencode and /oc forms. +# than only the bare /opencode and /oc forms. It also excludes a following +# "?": a query string glued directly onto the alias with no separator +# (/oc?mode=docs, /opencode?next=x) is a URL path with a query component, +# not a standalone command, and shares the exact same "alias text is a +# complete match, but something non-word continues right after it" shape. MENTION_PATTERNS = { "cwl-noema-review": re.compile( r"(? None: assert "opencode-agent" not in module.exact_mentions(body) +@pytest.mark.parametrize( + "body", + [ + "/oc?mode=docs", + "/opencode?next=x", + ], +) +def test_exact_mentions_rejects_trailing_query_string(body: str) -> None: + """A query string glued directly onto the alias is not a mention. + + Seventh-round finding on #1537's successor PR (CodeRabbit): the shared + trailing boundary excluded a following letter, digit, underscore, + hyphen, or slash, but not a following ``?``, so a query string with no + separator (``/oc?mode=docs``, ``/opencode?next=x``) still matched as a + complete mention — the same "alias text is a complete match, but + something non-word continues right after it" shape as the sixth-round + trailing-slash finding, just with ``?`` instead of ``/``. The fix adds + ``?`` to the same shared trailing exclusion. + """ + + module = load_module() + assert "opencode-agent" not in module.exact_mentions(body) + + @pytest.mark.parametrize( "payload", [ From de821dff291883789c1c77e5f5b8075d68eda53f Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 1 Sep 2026 07:04:13 +0000 Subject: [PATCH 05/12] fix(agent-mention): give each mention alternative its own boundary, fix a regression MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two real findings on this exact head (both Devin): 1. (New bug) The bare /opencode and /oc forms' boundary excluded neither a preceding "#" (a URL fragment identifier: https://example.com/#/oc) nor a following "." (a dotted filename continuation: /oc.json), and the trailing boundary used a plain ASCII character class, so a following non-ASCII word character was never excluded (/océan, where "é" is a Unicode letter outside [A-Za-z0-9_/?-]). 2. (Regression, introduced by the immediately preceding commit) Adding "?" to the shared trailing lookahead affected all three opencode-agent alternatives, not just the bare-slash forms it was meant for, so "@opencode-agent?" and "@cwl-noema-review/@opencode-agent?" — ordinary sentence punctuation, not a URL query string — stopped dispatching. Root cause of both: a single trailing lookahead shared across the whole alternation can't express "exclude ? only for these two alternatives, exclude . and # only for these two, but not those." Fixed by giving each alternative its own leading and trailing lookaround instead of one shared lookahead outside the group, and switching every boundary in this module from an ASCII-only character class to Python's Unicode-aware `\w` (which also closes the same class of gap for the @-mention forms, e.g. "café@opencode-agent", not just the reported bare-slash cases). While redesigning, also closed a previously-flagged, out-of-scope gap: `cwl-noema-review`'s own pattern didn't exclude a preceding "/", so "docs/@cwl-noema-review/@opencode-agent" still fired Noema (flagged but not fixed in an earlier round on this same PR chain, since fixing it then would have needed exactly this per-alternative-boundary work to avoid breaking the "@cwl-noema-review/@opencode-agent" separator's own recognition of the Noema mention preceding it). Verified with a from-scratch script against all ~35 accumulated accept/ reject cases from every prior round on this file before touching source, catching two design mistakes (a leftover shared trailing "/" that broke the separator form, and a dropped hyphen exclusion) before they ever reached the test suite. Full suite: 2285 passed, 1 skipped, 21 subtests passed. 100% coverage and 100% docstrings maintained. git diff --check clean. --- scripts/ci/agent_mention_router.py | 79 +++++++++++++++------------ tests/test_agent_mention_router.py | 88 ++++++++++++++++++++++++++---- 2 files changed, 123 insertions(+), 44 deletions(-) diff --git a/scripts/ci/agent_mention_router.py b/scripts/ci/agent_mention_router.py index da2f257826..0fbcadff2d 100755 --- a/scripts/ci/agent_mention_router.py +++ b/scripts/ci/agent_mention_router.py @@ -21,47 +21,58 @@ # 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. +# Boundary model (each alternative below carries its own leading and +# trailing lookaround, not a lookahead shared across the alternation, so +# each form's exclusions can differ where the false-positive classes +# differ): # -# The shared trailing boundary after any of the three alternatives also -# excludes a following "/": without it, a root-relative path continuation -# right after the alias (/oc/config, /opencode/docs, @opencode-agent/config, -# @cwl-noema-review/@opencode-agent/foo) still matched, since the alias text -# itself is a complete, valid match and nothing in the original trailing -# lookahead treated "/" as a word character. This mirrors the -# leading-boundary "/" exclusion already applied above and closes the same -# false-positive class from the trailing side, for every alternative rather -# than only the bare /opencode and /oc forms. It also excludes a following -# "?": a query string glued directly onto the alias with no separator -# (/oc?mode=docs, /opencode?next=x) is a URL path with a query component, -# not a standalone command, and shares the exact same "alias text is a -# complete match, but something non-word continues right after it" shape. +# - "@opencode-agent" and the combined "@cwl-noema-review/@opencode-agent" +# separator each exclude a preceding/following Unicode word character +# (\w — this also covers accented and other non-ASCII letters, not just +# ASCII), hyphen, or slash. The leading "/" exclusion rejects URL/path +# embedding (https://youtube.com/@opencode-agent, docs/@opencode-agent); +# the trailing "/" exclusion rejects a root-relative path glued directly +# onto the alias (@opencode-agent/config, +# @cwl-noema-review/@opencode-agent/foo). Ordinary sentence punctuation +# (a trailing "?", ".", "!") is deliberately NOT excluded here: a +# maintainer ending a sentence with "@opencode-agent?" is a legitimate +# request, not a URL continuation — rejecting it (an early version of +# this exclusion did, by mistake, when a query-string fix below was +# applied to every alternative instead of only the one it targeted) is a +# worse failure mode than never seeing the rare literal "@opencode-agent" +# immediately followed by junk with no separating space. +# - The "@cwl-noema-review/@opencode-agent" separator's own left boundary +# is on the combined literal as a whole, not just the trailing slash: a +# boundary check on the slash alone would still fire for invalid pasted +# text where "@cwl-noema-review" is itself embedded in a larger token +# (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"/"/oc" forms are the most URL/path-context-prone, +# so both sides exclude the full set of characters that continue a +# URL/path/filename token: a Unicode word character, ".", "/", "?", "=", +# "#", or "-". This rejects, on the leading side, a query string +# (?next=/opencode) and a URL fragment identifier +# (https://example.com/#/oc); and on the trailing side, a root-relative +# path (/oc/config), a dotted filename continuation (/oc.json), a query +# string glued on with no separator (/oc?mode=docs), and a Unicode word +# continuation (/océan) that a plain ASCII character class would miss. +# - "@cwl-noema-review" on its own additionally excludes a preceding "/" +# (closing the same URL/path-embedding class as "@opencode-agent" above) +# but deliberately NOT a trailing "/": that would break recognition of +# its own mention inside the "@cwl-noema-review/@opencode-agent" +# separator, where a "/" legitimately follows it. MENTION_PATTERNS = { "cwl-noema-review": re.compile( - r"(? None: ], ) def test_exact_mentions_rejects_trailing_query_string(body: str) -> None: - """A query string glued directly onto the alias is not a mention. - - Seventh-round finding on #1537's successor PR (CodeRabbit): the shared - trailing boundary excluded a following letter, digit, underscore, - hyphen, or slash, but not a following ``?``, so a query string with no - separator (``/oc?mode=docs``, ``/opencode?next=x``) still matched as a - complete mention — the same "alias text is a complete match, but - something non-word continues right after it" shape as the sixth-round - trailing-slash finding, just with ``?`` instead of ``/``. The fix adds - ``?`` to the same shared trailing exclusion. + """A query string glued directly onto the bare slash alias is not a mention. + + Seventh-round finding on #1537's successor PR (CodeRabbit): the bare + ``/opencode``/``/oc`` forms' trailing boundary excluded a following + letter, digit, underscore, hyphen, or slash, but not a following ``?``, + so a query string with no separator (``/oc?mode=docs``, + ``/opencode?next=x``) still matched as a complete mention. The fix adds + ``?`` to that alternative's own trailing exclusion only — see + ``test_exact_mentions_accepts_ordinary_punctuation_after_at_mentions`` + for why this must NOT be shared with the ``@``-mention alternatives. """ module = load_module() assert "opencode-agent" not in module.exact_mentions(body) +def test_exact_mentions_accepts_ordinary_punctuation_after_at_mentions() -> None: + """A trailing "?" after an @-mention is ordinary punctuation, not a mention. + + Ninth-round finding on #1537's successor PR (Devin), a regression from + the eighth-round fix above: excluding a trailing ``?`` was applied to + the whole ``opencode-agent`` alternation instead of scoped to only the + bare-slash forms it was meant for, so a maintainer ending a sentence + with ``@opencode-agent?`` (or the ``@cwl-noema-review/@opencode-agent`` + separator followed by ``?``) silently stopped dispatching. Each + alternative now carries its own trailing lookahead instead of one + shared across the alternation, so the bare-slash forms' ``?`` exclusion + no longer leaks onto the ``@``-mention forms. + """ + + module = load_module() + assert module.exact_mentions("@opencode-agent?") == ("opencode-agent",) + assert module.exact_mentions("@cwl-noema-review/@opencode-agent?") == ( + "cwl-noema-review", + "opencode-agent", + ) + + +@pytest.mark.parametrize( + "body", + [ + "https://example.com/#/oc", + "https://example.com/#/opencode", + "/oc.json", + "/opencode.json", + "/océan", + ], +) +def test_exact_mentions_rejects_fragment_dotted_and_unicode_continuations( + body: str, +) -> None: + """A URL fragment, dotted filename, or Unicode word continuation is not a mention. + + Eighth-round finding on #1537's successor PR (Devin): the bare + ``/opencode``/``/oc`` forms' boundary excluded neither a preceding + ``#`` (a URL fragment identifier, ``https://example.com/#/oc``) nor a + following ``.`` (a dotted filename continuation, ``/oc.json``), and + used a plain ASCII character class for the trailing boundary, which + does not exclude a following non-ASCII word character (``/océan``, + where ``é`` is a Unicode letter but not in ``[A-Za-z0-9_/?-]``). The fix + adds ``#`` and ``.`` to that alternative's own leading/trailing + exclusion set and switches every boundary in this module from an + ASCII-only character class to Python's Unicode-aware ``\\w``. + """ + + module = load_module() + assert "opencode-agent" not in module.exact_mentions(body) + + +def test_exact_mentions_rejects_unicode_embedded_at_mentions() -> None: + """A Unicode word character directly touching an @-mention is not a mention. + + Companion case to the eighth-round Unicode finding above, for the + ``@``-mention alternatives rather than the bare-slash forms: switching + their boundaries to Unicode-aware ``\\w`` closes the same class of gap + (a preceding or following accented letter that a plain ASCII character + class would not have excluded). + """ + + module = load_module() + assert module.exact_mentions("café@opencode-agent") == () + assert module.exact_mentions("@opencode-agenté") == () + + @pytest.mark.parametrize( "payload", [ From f26b3bdbd6128ef3dae53bedf0f39c305a9b0144 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 1 Sep 2026 07:13:20 +0000 Subject: [PATCH 06/12] fix(agent-mention): exclude a percent-encoded path or URI scheme separator The bare /opencode and /oc forms' boundary excluded neither a following "%" (a percent-encoded path continuation: /oc%2Fconfig) nor a preceding ":" (a URI scheme separator: scheme:/oc, app:/opencode), so both still matched as complete, standalone mentions. Reported by Devin on this PR's current head. Reproduced first, then added "%" and ":" to that alternative's own leading/trailing exclusion set alongside the ".", "/", "?", "=", and "#" already excluded there. Verified against the full accumulated accept/reject matrix (41 cases from every prior round on this file) before applying. Full suite: 2289 passed, 1 skipped, 21 subtests passed. 100% coverage and 100% docstrings maintained. --- scripts/ci/agent_mention_router.py | 16 +++++++++------- tests/test_agent_mention_router.py | 28 ++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 7 deletions(-) diff --git a/scripts/ci/agent_mention_router.py b/scripts/ci/agent_mention_router.py index 0fbcadff2d..590e55ff68 100755 --- a/scripts/ci/agent_mention_router.py +++ b/scripts/ci/agent_mention_router.py @@ -51,12 +51,14 @@ # - The bare "/opencode"/"/oc" forms are the most URL/path-context-prone, # so both sides exclude the full set of characters that continue a # URL/path/filename token: a Unicode word character, ".", "/", "?", "=", -# "#", or "-". This rejects, on the leading side, a query string -# (?next=/opencode) and a URL fragment identifier -# (https://example.com/#/oc); and on the trailing side, a root-relative -# path (/oc/config), a dotted filename continuation (/oc.json), a query -# string glued on with no separator (/oc?mode=docs), and a Unicode word -# continuation (/océan) that a plain ASCII character class would miss. +# "#", "%", ":", or "-". This rejects, on the leading side, a query +# string (?next=/opencode), a URL fragment identifier +# (https://example.com/#/oc), and a URI scheme separator (scheme:/oc, +# app:/opencode); and on the trailing side, a root-relative path +# (/oc/config), a dotted filename continuation (/oc.json), a query +# string glued on with no separator (/oc?mode=docs), a percent-encoded +# path continuation (/oc%2Fconfig), and a Unicode word continuation +# (/océan) that a plain ASCII character class would miss. # - "@cwl-noema-review" on its own additionally excludes a preceding "/" # (closing the same URL/path-embedding class as "@opencode-agent" above) # but deliberately NOT a trailing "/": that would break recognition of @@ -71,7 +73,7 @@ r"(?:" r"(? None: + """A percent-encoded path or URI-scheme-separated alias is not a mention. + + Tenth-round finding on #1537's successor PR (Devin): the bare + ``/opencode``/``/oc`` forms' boundary excluded neither a following + ``%`` (a percent-encoded path continuation, ``/oc%2Fconfig``) nor a + preceding ``:`` (a URI scheme separator, ``scheme:/oc``, ``app:/oc``), + so both still matched as complete, standalone mentions. The fix adds + ``%`` and ``:`` to that alternative's own leading/trailing exclusion + set, alongside the ``.``, ``/``, ``?``, ``=``, and ``#`` already + excluded there. + """ + + module = load_module() + assert "opencode-agent" not in module.exact_mentions(body) + + @pytest.mark.parametrize( "payload", [ From db106d50f2134ece147bc5318e389aeb124d198c Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 1 Sep 2026 07:21:06 +0000 Subject: [PATCH 07/12] test(ci): close main's post-#1546 scheduler coverage regression Protected main regressed to 99% scripts/ci coverage after #1546 added live_head_matches, a no-active/no-stale fall-through in prepare_autofix_slot, and an "already queued or running" wait branch to pr_review_fix_scheduler.py without covering them, while the pre-existing inspect_pr conflicted-draft/conflicted-unauthorized returns and pr_review_merge_scheduler.py's fetch_workflow_names_by_check_suite_rest pagination/filtering/ permission-denied paths stayed untested. Every PR rebasing onto main inherits this via the coverage-evidence required check regardless of its own diff. Test-only change; no production code touched. --- CHANGELOG.md | 9 +++ tests/test_pr_review_fix_scheduler.py | 49 ++++++++++++++ ...ew_fix_scheduler_rest_workflow_identity.py | 67 +++++++++++++++++++ 3 files changed, 125 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f5810d5308..1c46c64657 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,15 @@ this file. The format follows Keep a Changelog, and versioned releases follow Semantic Versioning where the repository publishes a release. ## [Unreleased] +- Close a 99% `scripts/ci` coverage regression on protected main: merged #1546 added an + uncovered `live_head_matches` helper, an uncovered no-active/no-stale-runs fall-through in + `prepare_autofix_slot`, and an uncovered "current-head autofix run is already queued or + running" wait path in `pr_review_fix_scheduler.py::inspect_pr`, while the pre-existing + conflicted-draft and conflicted-unauthorized `inspect_pr` returns and the REST + `fetch_workflow_names_by_check_suite_rest` pagination/name-filtering/permission-denied paths + in `pr_review_merge_scheduler.py` remained untested. Every PR rebasing onto main inherited + this failure via the `coverage-evidence` required check regardless of its own diff; this adds + test-only coverage for all of the above with no production code change. - Avoid redundant merge-scheduler wakes when the trusted receipt predicate already finds a substantive exact-head OpenCode verdict. Missing, stale, or fallback-only evidence still dispatches review work, while receipt lookup or diff --git a/tests/test_pr_review_fix_scheduler.py b/tests/test_pr_review_fix_scheduler.py index 9860eeaec7..f6abd64b0f 100644 --- a/tests/test_pr_review_fix_scheduler.py +++ b/tests/test_pr_review_fix_scheduler.py @@ -177,6 +177,40 @@ def test_prepare_autofix_slot_preserves_new_head_workers_after_head_advance(monk workflow_repository=fix.DEFAULT_AUTOFIX_REPOSITORY, dry_run=False, ) is None + + +def test_prepare_autofix_slot_returns_directly_with_no_active_or_stale_runs(monkeypatch): + """An empty Actions run list needs no reconciliation and skips cancellation.""" + monkeypatch.setattr(fix, "run_json", lambda _args: {"workflow_runs": []}) + monkeypatch.setattr( + fix, + "force_cancel_workflow_runs", + lambda *_args: pytest.fail("no stale runs must not attempt cancellation"), + ) + + assert fix.prepare_autofix_slot( + "owner/repo", + make_pr(), + workflow=fix.DEFAULT_AUTOFIX_WORKFLOW, + workflow_repository=fix.DEFAULT_AUTOFIX_REPOSITORY, + dry_run=False, + ) is False + + +def test_live_head_matches_compares_case_insensitively_and_fails_closed(monkeypatch): + """Live head lookup normalizes case and rejects malformed or mismatched payloads.""" + head = "a" * 40 + + monkeypatch.setattr(fix, "run_json", lambda _args: {"head": {"sha": head.upper()}}) + assert fix.live_head_matches("owner/repo", make_pr(headRefOid=head)) + + monkeypatch.setattr(fix, "run_json", lambda _args: {"head": {"sha": "b" * 40}}) + assert not fix.live_head_matches("owner/repo", make_pr(headRefOid=head)) + + monkeypatch.setattr(fix, "run_json", lambda _args: {"nothead": {}}) + assert not fix.live_head_matches("owner/repo", make_pr(headRefOid=head)) + + def test_terminal_failed_check_triggers_rca_without_prior_opencode_review(): """Exact-head check evidence can start RCA without a circular review prerequisite.""" pr = make_pr( @@ -1329,6 +1363,21 @@ def test_fix_inspect_skip_wait_and_error_paths(monkeypatch): monkeypatch.setattr(fix, "issue_comments", lambda repo, number: [{"body": f"{fix.FIX_MARKER} head_sha={'a' * 40} epoch={int(time.time())} -->"}]) assert fix.inspect_pr("owner/repo", make_pr(), args) == ("wait", ("recent autofix marker exists for this head",)) + 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",), + ) + + monkeypatch.setattr(fix, "issue_comments", lambda repo, number: []) + monkeypatch.setattr(fix, "prepare_autofix_slot", lambda *_args, **_kwargs: True) + assert fix.inspect_pr("owner/repo", make_pr(), args) == ( + "wait", + ("current-head autofix run is already queued or running",), + ) + pr1 = make_pr(number=1) pr2 = make_pr(number=2) monkeypatch.setattr(fix, "fetch_open_prs", lambda repo, max_prs: [pr1, pr2]) diff --git a/tests/test_pr_review_fix_scheduler_rest_workflow_identity.py b/tests/test_pr_review_fix_scheduler_rest_workflow_identity.py index c24cfb05f9..f261ce5beb 100644 --- a/tests/test_pr_review_fix_scheduler_rest_workflow_identity.py +++ b/tests/test_pr_review_fix_scheduler_rest_workflow_identity.py @@ -154,3 +154,70 @@ def fake_api(path: str) -> Any: assert merge.is_strix_context(context) assert merge.strix_evidence_state(pr) == expected_state assert fix.current_head_failed_checks(pr) == () + + +def test_fetch_workflow_names_by_check_suite_rest_paginates_past_100( + monkeypatch: Any, +) -> None: + """A first page of exactly 100 runs must fetch a second page and merge both.""" + head_sha = "e" * 40 + page1 = [ + {"check_suite_id": i, "name": f"workflow-{i}"} for i in range(100) + ] + page2 = [{"check_suite_id": 100, "name": "workflow-100"}] + calls: list[str] = [] + + def fake_api(path: str) -> Any: + calls.append(path) + if path.endswith("page=1"): + return {"workflow_runs": page1} + if path.endswith("page=2"): + return {"workflow_runs": page2} + raise AssertionError(f"unexpected path {path}") + + monkeypatch.setattr(merge, "gh_api_json", fake_api) + + names = merge.fetch_workflow_names_by_check_suite_rest("owner/repo", head_sha) + + assert names == {i: f"workflow-{i}" for i in range(101)} + assert calls == [ + f"repos/owner/repo/actions/runs?head_sha={head_sha}&per_page=100&page=1", + f"repos/owner/repo/actions/runs?head_sha={head_sha}&per_page=100&page=2", + ] + + +def test_fetch_workflow_names_by_check_suite_rest_skips_entries_missing_suite_id_or_name( + monkeypatch: Any, +) -> None: + """A run with no check-suite id or a blank name must not populate the map.""" + head_sha = "f" * 40 + + def fake_api(path: str) -> Any: + return { + "workflow_runs": [ + {"check_suite_id": None, "name": "orphaned run"}, + {"check_suite_id": 900, "name": ""}, + {"check_suite_id": 901, "name": "kept run"}, + ] + } + + monkeypatch.setattr(merge, "gh_api_json", fake_api) + + names = merge.fetch_workflow_names_by_check_suite_rest("owner/repo", head_sha) + + assert names == {901: "kept run"} + + +def test_fetch_workflow_names_by_check_suite_rest_propagates_non_access_errors( + monkeypatch: Any, +) -> None: + """A page-fetch failure unrelated to integration access must fail closed.""" + head_sha = "0" * 40 + + def fake_api(path: str) -> Any: + raise RuntimeError("gh: HTTP 502 (exhausted retries)") + + monkeypatch.setattr(merge, "gh_api_json", fake_api) + + with pytest.raises(RuntimeError, match="HTTP 502"): + merge.fetch_workflow_names_by_check_suite_rest("owner/repo", head_sha) From c46445b02a04a76cf6a3d72d8ab6e00d7025bc6c Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 1 Sep 2026 07:22:52 +0000 Subject: [PATCH 08/12] fix(agent-mention): make % and : boundary exclusions direction-specific MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The tenth-round fix added both "%" and ":" to the SAME leading-and-trailing exclusion set for the bare /opencode and /oc forms, but each character is only ever a URL/path continuation indicator from the direction it actually appears in a URL: - "%" starts a percent-encoding escape (%2F), which only ever continues a path FORWARD (/oc%2Fconfig) — a preceding "%" (100%/oc) is not a URL-encoding pattern. - ":" separates a URI scheme from its path (scheme:/oc), which only ever precedes the alias — a following ":" (/oc:) is not a scheme separator. Excluding "%" on the leading side and ":" on the trailing side too had no motivating false-positive case and instead rejected ordinary usage (reported by Devin on this PR's current head): "/oc:" (colon as a label separator after the command) and "100%/oc" (a percentage immediately before a command, no space). Fixed by splitting the shared set into direction-specific leading (".", "/", "?", "=", "#", ":", plus \w and "-") and trailing (".", "/", "?", "=", "#", "%", plus \w and "-") exclusion sets. Verified against the full accumulated accept/reject matrix (43 cases from every prior round on this file) before applying. Also fixes an unrelated, pre-existing test-harness flake this branch inherited: tests/test_opencode_required_verdict_regression.py's fake `gh` script didn't drain stdin before exiting on the dispatches branch, so the upstream `jq -cn | gh api --input -` pipe intermittently received SIGPIPE under `set -o pipefail` (exit 141) — a timing race, not a production bug (the real `gh api --input -` does read its stdin). Root-caused and fixed the same way as the already-diagnosed instance of this flake elsewhere in this PR chain: `cat >/dev/null` before recording the dispatch. Verified with 20 consecutive clean runs of the previously-flaky test after the fix. Full suite: 2293 passed, 1 skipped, 21 subtests passed. 100% coverage and 100% docstrings maintained. --- scripts/ci/agent_mention_router.py | 32 +++++++++----- tests/test_agent_mention_router.py | 44 +++++++++++++++++-- ...st_opencode_required_verdict_regression.py | 1 + 3 files changed, 63 insertions(+), 14 deletions(-) diff --git a/scripts/ci/agent_mention_router.py b/scripts/ci/agent_mention_router.py index 590e55ff68..c19a4c1342 100755 --- a/scripts/ci/agent_mention_router.py +++ b/scripts/ci/agent_mention_router.py @@ -49,16 +49,28 @@ # docs/@cwl-noema-review/@opencode-agent) without checking that the # Noema mention has a valid left boundary of its own. # - The bare "/opencode"/"/oc" forms are the most URL/path-context-prone, -# so both sides exclude the full set of characters that continue a -# URL/path/filename token: a Unicode word character, ".", "/", "?", "=", -# "#", "%", ":", or "-". This rejects, on the leading side, a query -# string (?next=/opencode), a URL fragment identifier +# so both sides exclude the characters that continue a URL/path/filename +# token, but NOT the same set on both sides — each excluded character is +# only ever a continuation indicator from the direction it actually +# appears in a URL. Leading exclusion: a Unicode word character, ".", +# "/", "?", "=", "#", ":", or "-". This rejects a query string +# (?next=/opencode), a URL fragment identifier # (https://example.com/#/oc), and a URI scheme separator (scheme:/oc, -# app:/opencode); and on the trailing side, a root-relative path -# (/oc/config), a dotted filename continuation (/oc.json), a query -# string glued on with no separator (/oc?mode=docs), a percent-encoded -# path continuation (/oc%2Fconfig), and a Unicode word continuation -# (/océan) that a plain ASCII character class would miss. +# app:/opencode) — but NOT a preceding "%", since percent-encoding syntax +# is "%" followed by hex digits, never followed by a literal "/", so a +# leading "%" before "/oc" (100%/oc) is not a URL-encoding pattern and +# was, in an earlier version of this exclusion, wrongly rejected as one. +# Trailing exclusion: a Unicode word character, ".", "/", "?", "=", "#", +# "%", or "-". This rejects a root-relative path (/oc/config), a dotted +# filename continuation (/oc.json), a query string glued on with no +# separator (/oc?mode=docs), a percent-encoded path continuation +# (/oc%2Fconfig), and a Unicode word continuation (/océan) that a plain +# ASCII character class would miss — but NOT a trailing ":", since a +# colon is not itself a path/URL continuation character in this +# direction (unlike the scheme-separator case, which is a *preceding* +# colon), so excluding it on the trailing side too, in an earlier +# version, wrongly rejected ordinary usage like "/oc:" (a colon used as +# a label separator after the command, not as part of a URL). # - "@cwl-noema-review" on its own additionally excludes a preceding "/" # (closing the same URL/path-embedding class as "@opencode-agent" above) # but deliberately NOT a trailing "/": that would break recognition of @@ -73,7 +85,7 @@ r"(?:" r"(? None: + """A colon after, or a percent sign before, the alias is not a URL indicator. + + Eleventh-round finding on #1537's successor PR (Devin), a regression + from the tenth-round fix above: that fix added both ``%`` and ``:`` to + the *same* leading-and-trailing exclusion set, but each character is + only ever a URL/path continuation indicator from the direction it + actually appears in a URL — a percent sign starts percent-encoding + escapes (``%2F``), so it only matters as a *trailing* character + (``/oc%2Fconfig``); a colon separates a URI scheme from its path, so it + only matters as a *leading* character (``scheme:/oc``). Excluding "%" + on the leading side and ":" on the trailing side too had no motivating + false-positive case and instead rejected ordinary usage: ``/oc:`` (a + colon used as a label separator after the command) and ``100%/oc`` (a + percentage immediately before a command, no space). The fix splits the + two into direction-specific exclusion sets instead of one shared set + applied to both boundaries. + """ + + module = load_module() + assert module.exact_mentions(body) == ("opencode-agent",) + + @pytest.mark.parametrize( "payload", [ diff --git a/tests/test_opencode_required_verdict_regression.py b/tests/test_opencode_required_verdict_regression.py index 8f8047ff10..0e5d30805b 100644 --- a/tests/test_opencode_required_verdict_regression.py +++ b/tests/test_opencode_required_verdict_regression.py @@ -173,6 +173,7 @@ def test_scheduler_wake_reuses_trusted_receipt_predicate( elif [[ "$*" == *"/pulls/7/reviews"* ]]; then printf '[%s]' "$FAKE_REVIEWS" elif [[ "$*" == *"repos/ContextualWisdomLab/.github/dispatches"* ]]; then + cat >/dev/null printf 'dispatch\n' >>"$DISPATCH_CALLS" fi """, From 6920f07f1a3f451b20dde5dfd2e211c39fa73abf Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 1 Sep 2026 07:30:01 +0000 Subject: [PATCH 09/12] fix(agent-mention): reject colon- and percent-delimited path segments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Distinguishing "/oc:" (accept: a colon used as a label separator, at end of string or before a space) from "/oc:config" (reject: a colon-delimited path segment) needs more context than a single trailing character can express — both have ":" as the character immediately after the alias. Likewise "100%/oc" (accept: a percentage before a command) from "docs/%/oc" (reject: a literal "%" path segment) both have "%" as the character immediately before the alias. Reported by Devin on this PR's current head. Reproduced first (/oc:config and docs/%/oc both matched before the fix). Fixed by adding two fixed-width two-character lookarounds on top of the existing single-character exclusion sets, rather than widening those sets (which cannot distinguish the accept case from the reject case sharing the same immediate character): - `(? None: + """A colon-delimited or percent-delimited path segment is not a mention. + + Twelfth-round finding on #1537's successor PR (Devin): distinguishing + ``/oc:`` (accept) from ``/oc:config`` (reject), and ``100%/oc`` (accept) + from ``docs/%/oc`` (reject), needs more context than a single + leading/trailing character can express — in both accept cases the + punctuation sits at a natural boundary (end of string, or preceded by + an ordinary word/digit); in both reject cases the SAME punctuation + character is itself part of a path/URI structure (a colon immediately + followed by more path text, forming a colon-delimited segment; a + percent sign immediately preceded by a path separator, forming a + literal "%" path segment). The fix adds two fixed-width two-character + lookarounds — ``(? Date: Tue, 1 Sep 2026 07:38:38 +0000 Subject: [PATCH 10/12] docs(gap-baseline): record post-#1546 scheduler coverage regression Adds a dated traceability entry for the coverage gap this PR closes: root cause (#1546's uncovered additions plus the older #1547/#1551/ #1554 gap, neither of which merged or transfers evidence here), the fix and its verification, the resolved Devin false-positive on sub-clause coverage, and the known pre-existing SIGPIPE test flake left unremediated as out of scope. --- docs/product-technical-gap-baseline.md | 48 ++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/docs/product-technical-gap-baseline.md b/docs/product-technical-gap-baseline.md index 76d85b949b..812f068e34 100644 --- a/docs/product-technical-gap-baseline.md +++ b/docs/product-technical-gap-baseline.md @@ -2344,6 +2344,54 @@ contract assertion, and `docs/adr/0003-contextual-orchestrator-vendored-free-zdr "today" reference. Landed in the same PR (`#1463`) as the streaming revert, not split out, since the revert is unsafe without it. +## 2026-09-01 post-#1546 `scripts/ci` coverage regression on protected main: root-caused and closed + +**Context**: `#1546` (merged, exact head `5686de41660d51a7a7f22b8840dfa6ccfe5ff3f1`) reconciled +unbounded exact-head review agents and, as part of a 90-line expansion of +`scripts/ci/pr_review_fix_scheduler.py`, added a `live_head_matches` helper, a no-active/no-stale +fall-through branch in `prepare_autofix_slot`, and an "already queued or running" wait branch in +`inspect_pr` — none of which any test exercised directly. This compounded a narrower, older gap in +the same file (`inspect_pr`'s conflicted-draft and conflicted-unauthorized returns) and in +`scripts/ci/pr_review_merge_scheduler.py::fetch_workflow_names_by_check_suite_rest` (pagination, +missing-suite-id/blank-name filtering, non-access-error propagation), first found and attempted in +now-closed, unmerged `#1547`/`#1551`/`#1554` — none of whose evidence or diffs transferred here; +this pass re-derived the current gap from a clean `origin/main` clone rather than assuming those +predecessors were still accurate against `#1546`'s shifted line numbers and new branches. Verified +directly: `coverage report --show-missing` on unmodified `main` showed +`scripts/ci/pr_review_fix_scheduler.py` at 97% (missing 116-121, 459->466, 495, 503, 546) and +`scripts/ci/pr_review_merge_scheduler.py` at 99% (missing 1003, 1008->1005, 1012) — total repo-wide +99%, below the `pyproject.toml` `fail_under = 100` gate. Because `opencode-review-dispatch.yml`'s +`coverage-evidence` job measures the **merged** PR tree (base + head) and hard-fails below 100%, +every PR rebasing onto main inherited this failure regardless of its own diff — org-wide impact, +not scoped to one PR. + +**Fix**: `#1567` (test-only, no production code) adds direct unit coverage for `live_head_matches` +(case-insensitive match, mismatch, malformed-payload paths), `prepare_autofix_slot`'s empty-run +fall-through, the `inspect_pr` conflicted-draft/conflicted-unauthorized/already-queued cases, and +the `fetch_workflow_names_by_check_suite_rest` pagination/filtering/error-propagation paths. +Verified on the fix commit (`db106d50f2134ece147bc5318e389aeb124d198c`): `coverage run -m pytest +tests -q` (2251 passed, 1 skipped, 21 subtests), `coverage report` (repo-wide 100%, both files +individually 100% statement and 100% branch), `interrogate` (100.0%). + +**Devin Review raised a false positive on the fix itself**, claiming +`test_live_head_matches_compares_case_insensitively_and_fails_closed` left non-object-payload, +non-string-SHA, and wrong-length-SHA branches uncovered. Re-verified against the actual gate rather +than accepted at face value: `live_head_matches` has exactly one `if` statement (two arcs, both +exercised by the committed test), and its final `return (isinstance(...) and len(...) == 40 and +...)` is a single boolean expression with no `if`/`else` of its own — `coverage.py`'s branch mode +(what `fail_under = 100` actually measures here) tracks control-flow arcs between statements, not +sub-clause condition coverage within one expression. The cited cases are additional test +thoroughness, not something the gate is currently failing on; confirmed by a full-suite run on the +exact same head showing both files at 100% branch coverage with zero missing branches. Replied with +this evidence on the review thread and did not widen the PR's diff for a claim that does not hold +against this repo's own tooling. + +**One test in the full suite remains a known, pre-existing flake**, unrelated to this change: +`tests/test_opencode_required_verdict_regression.py::test_scheduler_wake_reuses_trusted_receipt_predicate` +intermittently exits 141 (SIGPIPE) under full-suite parallel load; reproduces identically on +unmodified `origin/main` and passes cleanly in file isolation. Not remediated here — out of scope +for a coverage-gap-only PR, and not itself a coverage regression. + ## 5. 실행 루프와 고객의 다음 행동 각 hourly pass는 아래 순서를 유지한다. From 4b42ce97dbda173563ab2e84317df9416ff4025d Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 1 Sep 2026 07:38:41 +0000 Subject: [PATCH 11/12] fix(agent-mention): widen the trailing colon exclusion to cover a slash too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The twelfth-round fix's trailing colon lookaround, (?!:\w), only rejected a colon immediately followed by a word character (/oc:config), so a colon immediately followed by a slash (/oc:/config, /oc://foo) still matched — exactly as much a path/URI structure as the word-character case, just missed because the lookaround checked for a word character specifically instead of "word character or slash". Reported by Devin on this PR's current head. Reproduced first (/oc:/config and /oc://foo both matched before the fix). Fixed by widening (?!:\w) to (?!:[\w/]), still leaving the true accept case ("/oc:" at end of string, or followed by a space or other non-word/non-slash text) untouched. Verified against the full accumulated accept/reject matrix (50 cases from every prior round on this file) before applying. Full suite: 2301 passed, 1 skipped, 21 subtests passed. 100% coverage and 100% docstrings maintained. --- scripts/ci/agent_mention_router.py | 8 ++++++-- tests/test_agent_mention_router.py | 29 +++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/scripts/ci/agent_mention_router.py b/scripts/ci/agent_mention_router.py index b766ea6c83..6466c90218 100755 --- a/scripts/ci/agent_mention_router.py +++ b/scripts/ci/agent_mention_router.py @@ -81,7 +81,11 @@ # the "100%/oc" percentage case above, where the percent sign is preceded # by a digit. Both use a fixed-width two-character lookaround instead of # widening the single-character sets above, which would have reopened -# one of the two cases each pair is meant to distinguish. +# one of the two cases each pair is meant to distinguish. The trailing +# colon lookaround excludes a following word character OR "/", not just +# a word character: a colon followed by a slash (/oc:/config, /oc://foo) +# is exactly as much a path/URI structure as a colon followed directly +# by a word, and checking only for a word character left this open. # - "@cwl-noema-review" on its own additionally excludes a preceding "/" # (closing the same URL/path-embedding class as "@opencode-agent" above) # but deliberately NOT a trailing "/": that would break recognition of @@ -96,7 +100,7 @@ r"(?:" r"(? None: + """A colon followed by a slash is a path/URI structure, not punctuation. + + Thirteenth-round finding on #1537's successor PR (Devin): the + twelfth-round fix's trailing colon lookaround, ``(?!:\\w)``, only + rejected a colon immediately followed by a word character + (``/oc:config``), so a colon immediately followed by a slash + (``/oc:/config``, ``/oc://foo``) still matched — exactly as much a + path/URI structure as the word-character case, just missed because the + lookaround checked for a word character specifically instead of "word + character or slash". The fix widens that lookaround to + ``(?!:[\\w/])``, still leaving the true accept cases (``/oc:`` at end + of string, or followed by a space or other non-word/non-slash text) + untouched. + """ + + module = load_module() + assert "opencode-agent" not in module.exact_mentions(body) + + @pytest.mark.parametrize( "payload", [ From 6f40a0637da94da60f43ca72086d27e1034e8bbc Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Tue, 1 Sep 2026 16:46:11 +0900 Subject: [PATCH 12/12] test(ci): document nested REST fixture helpers Raise scoped docstring coverage for the newly added scheduler REST regression helpers to 100% without changing test behavior or production code. --- tests/test_pr_review_fix_scheduler_rest_workflow_identity.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/test_pr_review_fix_scheduler_rest_workflow_identity.py b/tests/test_pr_review_fix_scheduler_rest_workflow_identity.py index f261ce5beb..4e36544061 100644 --- a/tests/test_pr_review_fix_scheduler_rest_workflow_identity.py +++ b/tests/test_pr_review_fix_scheduler_rest_workflow_identity.py @@ -168,6 +168,7 @@ def test_fetch_workflow_names_by_check_suite_rest_paginates_past_100( calls: list[str] = [] def fake_api(path: str) -> Any: + """Return deterministic paginated workflow-run fixtures.""" calls.append(path) if path.endswith("page=1"): return {"workflow_runs": page1} @@ -193,6 +194,7 @@ def test_fetch_workflow_names_by_check_suite_rest_skips_entries_missing_suite_id head_sha = "f" * 40 def fake_api(path: str) -> Any: + """Return workflow runs that exercise incomplete-identity filtering.""" return { "workflow_runs": [ {"check_suite_id": None, "name": "orphaned run"}, @@ -215,6 +217,7 @@ def test_fetch_workflow_names_by_check_suite_rest_propagates_non_access_errors( head_sha = "0" * 40 def fake_api(path: str) -> Any: + """Simulate a non-access REST failure that must propagate.""" raise RuntimeError("gh: HTTP 502 (exhausted retries)") monkeypatch.setattr(merge, "gh_api_json", fake_api)