mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-11-06 05:17:15 +00:00
fix(webhook): a refused subscribe exits nonzero
On top of the salvaged routed-profile lookup: every refusal in
`hermes webhook subscribe` (bad name/profile, secret reuse on rebind,
--deliver-only conflicts, unknown or ambiguous --cron-job) and the
webhook-disabled gate now return 1, and `hermes webhook` forwards the
handler's code to the process exit, so a script can tell no route was
written. Tests drive the real `hermes` entry point.
Pre-existing before #106742 (the default-profile resolve_job_ref call and
the exit-0 path are both at d04056d609); the wrong-profile lookup only
bites when --route-profile names a multiplexed secondary. Needs a live
retest on a multiplexed install with a cron job in a secondary profile.
Refs #139332
Refs #134889 (CRON-2)
Refs #140138
This commit is contained in:
1 parent
9e41ea28c8
commit
f517b927fa
3 files changed
+39
-12
No files matched your search
+1
-1
@@ -1845,7 +1845,7 @@ cmd_logout = _forward_command("cmd_logout", "hermes_cli.auth", "logout_command",
|
||||
cmd_auth = _forward_command("cmd_auth", "hermes_cli.auth_commands", "auth_command", doc='Manage pooled credentials.')
|
||||
cmd_status = _forward_command("cmd_status", "hermes_cli.status", "show_status", doc='Show status of all components.')
|
||||
cmd_cron = _forward_command("cmd_cron", "hermes_cli.cron", "cron_command", forward_return=True, doc='Cron job management.')
|
||||
cmd_webhook = _forward_command("cmd_webhook", "hermes_cli.webhook", "webhook_command", doc='Webhook subscription management.')
|
||||
cmd_webhook = _forward_command("cmd_webhook", "hermes_cli.webhook", "webhook_command", forward_return=True, doc='Webhook subscription management.')
|
||||
cmd_kanban = _forward_command("cmd_kanban", "hermes_cli.kanban", "kanban_command", forward_return=True, doc='Multi-profile collaboration board.')
|
||||
cmd_project = _forward_command("cmd_project", "hermes_cli.projects_cmd", "projects_command", forward_return=True, doc='Manage projects (named, multi-folder workspaces).')
|
||||
cmd_hooks = _forward_command("cmd_hooks", "hermes_cli.hooks", "hooks_command", doc='Shell-hook inspection and management.')
|
||||
|
||||
+12
-11
@@ -169,17 +169,18 @@ def webhook_command(args):
|
||||
return
|
||||
if not _is_webhook_enabled():
|
||||
print(_setup_hint())
|
||||
return
|
||||
return 1
|
||||
handler = _ACTIONS.get(sub)
|
||||
if handler is not None:
|
||||
handler(args)
|
||||
# A refusal returns 1 so scripts can tell no route was written (#139332).
|
||||
return handler(args)
|
||||
|
||||
|
||||
def _cmd_subscribe(args):
|
||||
name = args.name.strip().lower().replace(" ", "-")
|
||||
if not re.match(r'^[a-z0-9][a-z0-9_-]*$', name):
|
||||
print(f"Error: Invalid name '{name}'. Use lowercase alphanumeric with hyphens/underscores.")
|
||||
return
|
||||
return 1
|
||||
|
||||
subs = _load_subscriptions()
|
||||
expected = subs.get(name, _MISSING_SUBSCRIPTION)
|
||||
@@ -195,10 +196,10 @@ def _cmd_subscribe(args):
|
||||
validate_profile_name(profile)
|
||||
except ValueError as exc:
|
||||
print(f"Error: {exc}")
|
||||
return
|
||||
return 1
|
||||
if not profile_exists(profile):
|
||||
print(f"Error: Profile '{profile}' does not exist.")
|
||||
return
|
||||
return 1
|
||||
existing_profile = existing.get("profile", "default")
|
||||
profile_changed = is_update and profile != existing_profile
|
||||
if profile_changed and args.secret and args.secret == existing.get("secret"):
|
||||
@@ -206,7 +207,7 @@ def _cmd_subscribe(args):
|
||||
"Error: Rebinding a webhook route requires a new HMAC secret. "
|
||||
"Omit --secret to generate one automatically, or provide a different secret."
|
||||
)
|
||||
return
|
||||
return 1
|
||||
secret = args.secret or (None if profile_changed else existing.get("secret")) or secrets.token_urlsafe(32)
|
||||
events = [e.strip() for e in args.events.split(",")] if args.events else []
|
||||
route = {
|
||||
@@ -227,12 +228,12 @@ def _cmd_subscribe(args):
|
||||
"--cron-job fires an existing cron job (which handles its own "
|
||||
"delivery)."
|
||||
)
|
||||
return
|
||||
return 1
|
||||
if route["deliver"] == "log":
|
||||
print(
|
||||
"Error: --deliver-only requires --deliver to be a real target "
|
||||
"(telegram, discord, slack, github_comment, etc.) — not 'log'.")
|
||||
return
|
||||
return 1
|
||||
route["deliver_only"] = True
|
||||
if getattr(args, "mirror_to_session", False):
|
||||
route["mirror_to_session"] = True
|
||||
@@ -244,10 +245,10 @@ def _cmd_subscribe(args):
|
||||
job = _resolve_route_cron_job(profile, cron_job)
|
||||
except AmbiguousJobReference as e:
|
||||
print(f"Error: {e}")
|
||||
return
|
||||
return 1
|
||||
if job is None:
|
||||
print(f"Error: no cron job matches '{cron_job}'. List jobs with: hermes cron list")
|
||||
return
|
||||
return 1
|
||||
route["cron_job"] = job["id"]
|
||||
script = (getattr(args, "script", "") or "").strip()
|
||||
if script:
|
||||
@@ -258,7 +259,7 @@ def _cmd_subscribe(args):
|
||||
route = _replace_subscription(name, route, expected)
|
||||
except SubscriptionMutationConflict as exc:
|
||||
print(f"Error: {exc}")
|
||||
return
|
||||
return 1
|
||||
|
||||
print(f"\n {'Updated' if is_update else 'Created'} webhook subscription: {name}")
|
||||
print(f" URL: {_route_url(name, route)}")
|
||||
|
||||
@@ -94,3 +94,29 @@ def test_bare_route_preserves_custom_caller_home(homes, monkeypatch):
|
||||
subscribe()
|
||||
assert webhook._load_subscriptions()["route"]["cron_job"] == "custom-job"
|
||||
assert not (root / "webhook_subscriptions.json").exists()
|
||||
|
||||
|
||||
def _hermes(monkeypatch, *argv):
|
||||
"""Run the real ``hermes`` entry point; return the process exit code."""
|
||||
import sys
|
||||
from hermes_cli import main as hermes_main
|
||||
monkeypatch.setattr(sys, "argv", ["hermes", *argv])
|
||||
try:
|
||||
hermes_main.main()
|
||||
except SystemExit as exc:
|
||||
return exc.code or 0
|
||||
return 0
|
||||
|
||||
|
||||
def test_cli_refusal_exits_nonzero_and_writes_nothing(homes, monkeypatch):
|
||||
"""#139332: a refused subscribe exited 0, so a script could not tell no route was created."""
|
||||
root, _ = homes
|
||||
assert _hermes(monkeypatch, "webhook", "subscribe", "route", "--route-profile", "coder",
|
||||
"--cron-job", "no-such-job") == 1
|
||||
assert not (root / "webhook_subscriptions.json").exists()
|
||||
|
||||
|
||||
def test_cli_routed_cron_subscribe_exits_zero(homes, monkeypatch):
|
||||
assert _hermes(monkeypatch, "webhook", "subscribe", "route", "--route-profile", "coder",
|
||||
"--cron-job", "coder-job") == 0
|
||||
assert webhook._load_subscriptions()["route"]["cron_job"] == "coder-job"
|
||||
Reference in new issue
Block a user