Skip to content

fix(godcmd): answer when the plugin priority is not a number - #3303

Merged
zhayujie merged 1 commit into
zhayujie:masterfrom
Lesereingrape:fix/godcmd-setpri-priority
Sep 29, 2026
Merged

zhayujie merged 1 commit into
zhayujie:masterfrom
Lesereingrape:fix/godcmd-setpri-priority

Conversation

@Lesereingrape

Copy link
Copy Markdown
Contributor

What

#setpri (设置插件优先级) checked the argument count and then converted the second argument with a bare int(), so a priority typed as a word -- #setpri keyword high -- raised ValueError out of on_handle_context and the admin got no reply at all. The value is now parsed inside a try, an unparsable one is answered through the reply path the command already uses (ReplyType.ERROR, naming what was typed: 优先级 high 无效, 应为整数), and the numeric path is unchanged -- including the success text, which still echoes the argument the user wrote.

Fixes #3302

Why it was silent rather than an error message

Nothing on the way back catches that exception: PluginManager.emit_event invokes the handler directly (plugins/plugin_manager.py:287-297), ChatChannel._handle has no try/except around it (channel/chat_channel.py:191-209), and the worker's exception callback only logs it (channel/chat_channel.py:452-453). So the one command that takes a number from the user is the one command that cannot answer a wrong one: reloadp (line 404), enablep (413), disablep (418) and installp (427) all reply on a bad argument, and #setpri already replies on a bad argument count -- only the value was unguarded.

Checks

  • RED on master, production code untouched -- pytest tests/test_godcmd_setpri_priority_argument.py fails on the first case with ValueError: invalid literal for int() with base 10: 'high' at plugins/godcmd/godcmd.py:397, i.e. the handler never reaches the reply it is supposed to build.
  • GREEN after the change: 3 passed.
  • Whole suite on the same tip, before and after: the FAILED set is identical (32 pre-existing failures, all environment-bound on Windows -- scheduler timezone, symlink pruning, the rg backend parity case, /proc/self/environ probes). passed goes 2256 -> 2259, exactly the three new cases. No other file was touched.
  • Repo's own lint gate (ruff check --select F --ignore F403,F405) on both changed files: All checks passed.

The new tests stub Bridge and PluginManager, so no network and no plugin config is written; the plugin itself is pointed at a tmp_path config carrying admin_users.

#setpri checked the argument count and then read the second argument with a
bare int(), so "#setpri keyword high" raised ValueError out of
on_handle_context. Nothing downstream catches it -- emit_event calls the
handler directly and the worker callback only logs -- so the admin got no
reply of any kind instead of the usage text every sibling command answers.
@zhayujie
zhayujie merged commit 6876520 into zhayujie:master Sep 29, 2026
3 checks passed
@zhayujie

Copy link
Copy Markdown
Owner

Merged, thanks! A typo in the priority now gets a clear answer, same as #3291 did for Midjourney.

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.

godcmd: #setpri with a non-numeric priority raises ValueError and answers nothing

2 participants