Skip to content

fix: normalize Skill tool args to prevent TypeError when models pass objects - #186

Open
linhongyu510 wants to merge 1 commit into
SAIL-Research-Lab:mainfrom
linhongyu510:fix/skill-tool-args-normalization
Open

linhongyu510 wants to merge 1 commit into
SAIL-Research-Lab:mainfrom
linhongyu510:fix/skill-tool-args-normalization

Conversation

@linhongyu510

Copy link
Copy Markdown

Problem

When a model calls the Skill tool with args as an object instead of a string, the tool crashes:

Error executing Skill: replace() argument 2 must be str, not dict

Observed tool call (from a saved session, mr_sessions/session_latest.json):

{"name": "Skill", "input": {"name": "recall", "args": {"args": "--corpus \"$MEMORY_CORPUS\" --k 5"}}}

_skill_tool does args = params.get("args", "") with no type check and hands the value to substitute_arguments, which calls prompt.replace("$ARGUMENTS", args) → TypeError. The raw traceback is surfaced verbatim to the model, which retries the identical call and eventually gives up on the skill and hallucinates the answer (observed: 4 identical retries with gpt-oss-120b via llama-server). Small/open models on custom providers wrap tool input in an object fairly often because every tool appears to them as input: {...}.

Fix

Add _normalize_args() in cheetahclaws/skill/tools.py and call it at the top of _skill_tool:

  • str → unchanged.
  • dict with a single string value (the observed {"args": "..."} shape) → unwrap the value.
  • list of strings (e.g. ["--env", "prod"]) → join with spaces.
  • Anything else (multi-key dict, int, None, …) → return a concrete Error: Skill 'args' must be a string… message with a suggested shape, so the model gets something it can act on instead of a traceback.

Tests

4 new tests in tests/test_skills.py (all fail on main, pass here):

  • dict with a single string value is unwrapped and the skill runs;
  • a list of strings is joined and the skill runs;
  • a multi-key dict returns an error containing "must be a string" (no crash);
  • a non-string (int) returns an error containing "must be a string" (no crash).

The shared skill_dir fixture now also writes the existing deploy skill (the ARGS_MD fixture constant already defined in the file) so the new tests exercise a prompt containing $ARGUMENTS; test_load_skills count assertion updated 2 → 3 accordingly.

Verification: tests/test_skills.py 30 passed (26 pre-existing + 4 new). Full non-e2e suite: 2782 passed; the 5 failures and 33 web-api import errors are environment-related and reproduce identically on main (macOS rlimit/case-sensitivity, missing SQLAlchemy for the web extra). ruff check on the two touched files reports only the 6 pre-existing findings — no new lint issues.

Honest caveat: the tool's agent run is mocked in tests (_fake_run), so the fix is verified at the unit level; no live model run was performed.

…objects

Small/open models on custom providers often wrap tool input in an
object ({'args': {...}}) or a list. _skill_tool handed the raw value
to substitute_arguments, which called prompt.replace("", args)
and crashed with 'TypeError: replace() argument 2 must be str, not
dict' — surfaced verbatim to the model, which retried the same call
and gave up on the skill (SAIL-Research-Lab#182).

Add _normalize_args(): unwrap a single-string-value dict, join a list
of strings, and return a concrete correction for anything else so the
model gets an actionable error instead of a traceback.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant