From a1264e9967ed4cbf4b6a60809628c197053bdf42 Mon Sep 17 00:00:00 2001 From: alt-glitch Date: Wed, 3 Jun 2026 11:41:09 +0000 Subject: [PATCH] fix(matrix): make bang-command resolution robust + fix dead skill-command branch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to the salvaged contributor commit: - Underscore→hyphen tolerance now emits a resolvable token. Previously the detect set accepted the hyphenated variant but emit returned the raw token, so '!set_home' produced '/set_home' which the dispatcher could not resolve. Now emits '/set-home'. Aliases are left as-is — the gateway dispatcher canonicalizes them itself. - Fix dead skill-command branch: skill command keys are stored slash-prefixed (e.g. '/arxiv') in get_skill_commands(), but the check compared the bare token, so '!arxiv' never normalized. Now compares the '/candidate' form, making skill aliases (e.g. !gif-search) work. - Re-run bang normalization after Matrix reply-fallback stripping so a quoted reply whose content is a bang command reaches command parity with the slash form. - Replace silent 'except Exception: pass' with logger.debug(exc_info=True). - Add AUTHOR_MAP entry for @nepenth. Tests: +5 (underscore-alias, skill-command branch, quoted-reply bang + slash parity). 162 Matrix tests pass. --- gateway/platforms/matrix.py | 53 ++++++++++++++++------ scripts/release.py | 1 + tests/gateway/test_matrix.py | 86 ++++++++++++++++++++++++++++++++++++ 3 files changed, 126 insertions(+), 14 deletions(-) diff --git a/gateway/platforms/matrix.py b/gateway/platforms/matrix.py index 7b7df2d58..cccb4d70e 100644 --- a/gateway/platforms/matrix.py +++ b/gateway/platforms/matrix.py @@ -113,35 +113,55 @@ _MATRIX_BANG_COMMAND_RE = re.compile( ) -def _is_known_matrix_bang_command(name: str) -> bool: - """Return True when *name* is a Hermes command worth normalizing. +def _resolve_matrix_bang_command(name: str) -> str | None: + """Resolve a ``!command`` token to a dispatchable Hermes command token. Matrix clients often reserve leading ``/`` for local client commands. Hermes accepts ``!command`` as a Matrix-friendly alias, but only for commands that the gateway can actually dispatch so ordinary exclamations remain normal chat text. + + Returns the token form that actually resolves (which may differ from + *name* only by underscore→hyphen normalization, e.g. ``reload_skills`` → + ``reload-skills``) so the emitted ``/command`` always resolves downstream, + or ``None`` when *name* is not a known command. Aliases are intentionally + left as-is — the gateway dispatcher resolves them to their canonical name. """ if not name: - return False - candidates = {name.lower(), name.lower().replace("_", "-")} + return None + # Try the raw lowercased token first, then its hyphenated variant, so + # forms like ``!reload_skills`` resolve against ``reload-skills``. We emit + # whichever candidate resolved (not a forced canonical form) to preserve + # alias passthrough — the gateway dispatcher canonicalizes aliases itself. + candidates = [name.lower()] + hyphenated = name.lower().replace("_", "-") + if hyphenated != candidates[0]: + candidates.append(hyphenated) + try: from hermes_cli.commands import is_gateway_known_command - if any(is_gateway_known_command(candidate) for candidate in candidates): - return True + for candidate in candidates: + if is_gateway_known_command(candidate): + return candidate except Exception: - pass + logger.debug( + "Matrix: is_gateway_known_command failed for %r", name, exc_info=True + ) try: from agent.skill_commands import get_skill_commands skill_commands = get_skill_commands() or {} - if any(candidate in skill_commands for candidate in candidates): - return True + # Skill command keys are stored slash-prefixed (e.g. "/arxiv"), so + # compare against the "/candidate" form, not the bare token. + for candidate in candidates: + if f"/{candidate}" in skill_commands: + return candidate except Exception: - pass + logger.debug("Matrix: get_skill_commands failed for %r", name, exc_info=True) - return False + return None def _normalize_matrix_bang_command(text: str) -> str: @@ -151,10 +171,10 @@ def _normalize_matrix_bang_command(text: str) -> str: match = _MATRIX_BANG_COMMAND_RE.match(text) if not match: return text - command = match.group(1).lower() - if not _is_known_matrix_bang_command(command): + resolved = _resolve_matrix_bang_command(match.group(1)) + if resolved is None: return text - return f"/{command}{match.group(2) or ''}" + return f"/{resolved}{match.group(2) or ''}" @dataclass @@ -1901,6 +1921,11 @@ class MatrixAdapter(BasePlatformAdapter): stripped.append(line) body = "\n".join(stripped) if stripped else body + # Re-run bang normalization after reply-fallback stripping so a quoted + # reply whose actual content is a bang command (e.g. ``> quoted\n\n!model``) + # is treated as a command, matching how ``/command`` is recognized below. + body = _normalize_matrix_bang_command(body) + msg_type = MessageType.TEXT if body.startswith("/"): msg_type = MessageType.COMMAND diff --git a/scripts/release.py b/scripts/release.py index c2b30655e..924e12f1c 100755 --- a/scripts/release.py +++ b/scripts/release.py @@ -46,6 +46,7 @@ ACP_REGISTRY_MANIFEST = REPO_ROOT / "acp_registry" / "agent.json" # Auto-extracted from noreply emails + manual overrides AUTHOR_MAP = { "liliangjya@gmail.com": "truenorth-lj", + "16943149+nepenth@users.noreply.github.com": "nepenth", "ben.bartholomew@vectorize.io": "benfrank241", "74339271+SaguaroDev@users.noreply.github.com": "SaguaroDev", "subw3@mail2.sysu.edu.cn": "Subway2023", diff --git a/tests/gateway/test_matrix.py b/tests/gateway/test_matrix.py index 4f43ace70..00b100d5a 100644 --- a/tests/gateway/test_matrix.py +++ b/tests/gateway/test_matrix.py @@ -567,6 +567,30 @@ class TestMatrixBangCommandAlias: ) return captured_event + async def _dispatch_text_reply(self, body: str, *, is_dm: bool = True): + """Dispatch a message that is a Matrix reply (m.in_reply_to set), so + the reply-fallback quote stripping path runs before command detection. + """ + captured_event = None + self.adapter._is_dm_room = AsyncMock(return_value=is_dm) + self.adapter._require_mention = True + self.adapter._free_rooms = set() + + async def capture(msg_event): + nonlocal captured_event + captured_event = msg_event + + self.adapter.handle_message = capture + await self.adapter._handle_text_message( + room_id="!room:example.org", + sender="@alice:example.org", + event_id="$matrix-reply-command-test", + event_ts=0.0, + source_content={"msgtype": "m.text", "body": body}, + relates_to={"m.in_reply_to": {"event_id": "$parent-event"}}, + ) + return captured_event + def test_known_bang_command_normalizes_to_slash_command(self): from gateway.platforms.matrix import _normalize_matrix_bang_command @@ -639,6 +663,68 @@ class TestMatrixBangCommandAlias: assert captured_event is None + def test_bang_alias_underscore_resolves_to_hyphen_form(self): + """!set_home must emit a dispatchable token even though set_home is + not itself registered — the hyphenated alias set-home is.""" + from gateway.platforms.matrix import _normalize_matrix_bang_command + + # set_home (underscore) is NOT a registered command/alias, but + # set-home (hyphen) is. The normalizer must emit the resolvable form. + assert _normalize_matrix_bang_command("!set_home") == "/set-home" + # The hyphen alias passes through unchanged. + assert _normalize_matrix_bang_command("!set-home") == "/set-home" + # The canonical command resolves directly. + assert _normalize_matrix_bang_command("!sethome") == "/sethome" + + def test_bang_skill_command_normalizes(self): + """The get_skill_commands() branch normalizes installed skill + commands, not just built-in gateway commands. Skill keys are stored + slash-prefixed (e.g. "/arxiv"), which the resolver must account for.""" + import agent.skill_commands as skill_commands_mod + + fake_skills = {"/arxiv": {}, "/obsidian": {}} + with patch.object( + skill_commands_mod, "get_skill_commands", return_value=fake_skills + ): + from gateway.platforms.matrix import _normalize_matrix_bang_command + + # is_gateway_known_command won't know these; the skill branch must. + assert _normalize_matrix_bang_command("!arxiv") == "/arxiv" + assert ( + _normalize_matrix_bang_command("!obsidian search foo") + == "/obsidian search foo" + ) + # A name in neither registry stays plain text. + assert ( + _normalize_matrix_bang_command("!definitelynotacommand") + == "!definitelynotacommand" + ) + + @pytest.mark.asyncio + async def test_bang_command_in_quoted_reply_normalizes(self): + """A bang command that follows a Matrix reply-fallback quote is + normalized after the quote is stripped, matching /command behavior.""" + captured_event = await self._dispatch_text_reply( + "> <@bob:example.org> earlier message\n\n!model" + ) + + assert captured_event is not None + assert captured_event.text == "/model" + assert captured_event.message_type == MessageType.COMMAND + assert captured_event.get_command() == "model" + + @pytest.mark.asyncio + async def test_slash_command_in_quoted_reply_normalizes(self): + """Sanity: the slash equivalent already works post-strip — the bang + form above must reach parity with this.""" + captured_event = await self._dispatch_text_reply( + "> <@bob:example.org> earlier message\n\n/model" + ) + + assert captured_event is not None + assert captured_event.text == "/model" + assert captured_event.message_type == MessageType.COMMAND + # --------------------------------------------------------------------------- # Thread detection