From c83c63f3feb684d7af692bcb7e04cc4b380bbd6c Mon Sep 17 00:00:00 2001 From: Daniel Pike Date: Thu, 23 Apr 2026 17:42:33 -0400 Subject: [PATCH] fix: return None when skill payload fails to load (#14713) build_skill_invocation_message no longer returns a truthy error string that the CLI treated as successful input. Log a warning and return None so callers surface errors instead of queueing a sentinel. Gateway returns a user-visible message when a resolved skill fails to load. TUI gateway returns 5030 for skill and /plan load failures. Tests: regression for None on _load_skill_payload failure. Made-with: Cursor --- agent/skill_commands.py | 12 ++++++++++-- gateway/run.py | 6 ++++++ tests/agent/test_skill_commands.py | 9 +++++++++ tui_gateway/server.py | 6 ++++++ 4 files changed, 31 insertions(+), 2 deletions(-) diff --git a/agent/skill_commands.py b/agent/skill_commands.py index 9c130ab84afc7..9ceeefc15a798 100644 --- a/agent/skill_commands.py +++ b/agent/skill_commands.py @@ -439,7 +439,9 @@ def build_skill_invocation_message( user_instruction: Optional text the user typed after the command. Returns: - The formatted message string, or None if the skill wasn't found. + The formatted message string, or ``None`` if the skill was not found + or the skill payload could not be loaded (callers must not queue + ``None`` as user input; see #14713). """ commands = get_skill_commands() skill_info = commands.get(cmd_key) @@ -448,7 +450,13 @@ def build_skill_invocation_message( loaded = _load_skill_payload(skill_info["skill_dir"], task_id=task_id) if not loaded: - return f"[Failed to load skill: {skill_info['name']}]" + logger.warning( + "Skill registered at %s but payload failed to load (name=%r); " + "not sending a synthetic user message (#14713)", + cmd_key, + skill_info.get("name"), + ) + return None loaded_skill, skill_dir, skill_name = loaded activation_note = ( diff --git a/gateway/run.py b/gateway/run.py index 3eb932cc24d10..253d7706ff965 100644 --- a/gateway/run.py +++ b/gateway/run.py @@ -3820,6 +3820,12 @@ async def _handle_message(self, event: MessageEvent) -> Optional[str]: if msg: event.text = msg # Fall through to normal message processing with skill content + else: + _disp = _skill_name or command + return ( + f"Failed to load skill **{_disp}**. " + "Check SKILL.md and any required setup, then try again." + ) else: # Not an active skill — check if it's a known-but-disabled or # uninstalled skill and give actionable guidance. diff --git a/tests/agent/test_skill_commands.py b/tests/agent/test_skill_commands.py index bf8742690c363..d4b85a62f32ac 100644 --- a/tests/agent/test_skill_commands.py +++ b/tests/agent/test_skill_commands.py @@ -285,6 +285,15 @@ def test_returns_none_for_unknown(self, tmp_path): msg = build_skill_invocation_message("/nonexistent") assert msg is None + def test_returns_none_when_payload_load_fails(self, tmp_path): + """#14713: failed loads must not return a truthy sentinel queued as input.""" + with patch("tools.skills_tool.SKILLS_DIR", tmp_path): + _make_skill(tmp_path, "fragile-skill") + scan_skill_commands() + with patch("agent.skill_commands._load_skill_payload", return_value=None): + msg = build_skill_invocation_message("/fragile-skill", "task") + assert msg is None + def test_uses_shared_skill_loader_for_secure_setup(self, tmp_path, monkeypatch): monkeypatch.delenv("TENOR_API_KEY", raising=False) calls = [] diff --git a/tui_gateway/server.py b/tui_gateway/server.py index 165b47bf99cdf..bffe7f1034f94 100644 --- a/tui_gateway/server.py +++ b/tui_gateway/server.py @@ -3025,6 +3025,11 @@ def _(rid, params: dict) -> dict: "name": cmds[key].get("name", name), }, ) + return _err( + rid, + 5030, + f"Failed to load skill: {cmds[key].get('name', name)}", + ) except Exception: pass @@ -3111,6 +3116,7 @@ def _(rid, params: dict) -> dict: ) if msg: return _ok(rid, {"type": "send", "message": msg}) + return _err(rid, 5030, "Failed to load the bundled /plan skill") except Exception as e: return _err(rid, 5030, f"plan skill failed: {e}")