fix(matrix): make bang-command resolution robust + fix dead skill-command branch
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.
This commit is contained in:
alt-glitch
committed by
Siddharth Balyan
Siddharth Balyan
parent
0022e94d74
commit
a1264e9967
@ -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
|
||||
|
||||
@ -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",
|
||||
|
||||
@ -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
|
||||
|
||||
Reference in New Issue
Block a user