`scan_plugins` reloads every plugin it has already imported, and reloading
re-runs the plugin's `@PluginManager().register(...)` decorator, which builds
a brand new class object and writes it into the registry. The scan then
decided which plugins were new with `list(set(news) - set(raws))` over the
classes themselves -- an identity comparison that the reload guarantees to
differ.
So every scan reported every plugin it had just reloaded as newly installed.
`#scanp` (`plugins/godcmd/godcmd.py:389`) hands that list straight to the user
as "发现新插件", so asking what had just been installed produced an answer
naming the entire plugins directory, and the same thing happened on every
subsequent scan.
Compare the registry keys instead, which is what identity means here:
`register()` writes `name.upper()` into the registry, and a reload writes the
same key it wrote before.
`reload_plugin` dropped the plugin's entry from the in-memory config on the way
in, so the fresh instance would re-read its settings. That only ever worked for
a plugin carrying its own `config.json` in its bundle: `Plugin.load_config`
falls back to that file and never looks at `plugins/config.json`, which is
where a plugin configured at runtime actually lives. For such a plugin the
entry was simply gone and nothing put it back.
Measured through the real `PluginManager` and the real `Godcmd`:
before reload : {'password': 's3cret', 'admin_users': ['alice']}
reload_plugin returned: True
live instance : password='' admin_users=[] temp='9623'
`#reloadp Godcmd` answered "插件配置已重载" while clearing the admin password
and the admin list, and logged a random four-digit temporary password in their
place - the admin has locked themselves out until the process restarts. The
removal also ran before the membership check, so `#reloadp <a name that is not
loaded>` erased that plugin's settings before discovering there was nothing to
reload.
Reloading a plugin's code is not the operation that reloads its configuration;
`#reconf` is, and it is what a user reaches for after editing config.json. So
the reload keeps the configuration it already has.
`remove_plugin_config` had no other caller, so its import goes with it. The
function itself is left alone in `config.py` in case something outside this
repository imports it.
load_config sorted plugins.json with a bare v["priority"], and scan_plugins
reads ["enabled"] the same way, so one hand-edited entry that lost or
mis-typed a key raised out of the plugin load -- the failure the surrounding
guards for a truncated or shapeless store were written to prevent.
Replace the per-module sibling-file-and-rename copies with
common/atomic_write.py, which uses a unique dot-prefixed temp name,
fsyncs, and keeps the target's permissions.
Co-authored-by: cowagent <cow@cowagent.ai>
save_config wrote straight into plugins.json, so anything that failed partway
through -- a full disk, an update interrupted mid-copy -- left a truncated
document where the plugin states used to be. load_config then read it with a
bare json.load and no guard:
with open(cfg_path, "r", encoding="utf-8") as f:
pconf = json.load(f)
pconf["plugins"] = SortedDict(...)
A truncated file raises JSONDecodeError there; a file that still parses but lost
its "plugins" key raises KeyError on the very next line. Either one escapes
load_plugins, which app.py:252 calls on first start:
if first_start:
if DESKTOP_MODE:
threading.Thread(target=PluginManager().load_plugins, daemon=True).start()
else:
PluginManager().load_plugins()
On a source deployment that is a failed start. On the desktop client the same
raise happens on a daemon thread, so it is never caught and never logged: the
plugin system simply never comes up -- /status, /help and the rest stop
answering -- with nothing in the log to say why. And the store is not obscure:
enabled and priority live only there, and six call sites write it (enable,
disable, set priority, install, uninstall, first scan), so a routine click in
the console is enough to put it at risk.
save_config now builds the store in a sibling .tmp file and os.replace()s it in
-- the shape task_store.py:87 (_write_atomic), recipient_store.py:67
(_save_unlocked), channel/weixin/weixin_channel.py:80 (_save_credentials) and
channel/web/api/agents.py:422 already use. A failure removes the temp file and
re-raises, leaving the previous store exactly as it was.
load_config keeps the fallback it already documents ("prefer the writable copy,
fall back to the one shipped in the resource dir") but stops choosing between
the two with os.path.exists. A damaged file passes that check, which is exactly
the case the fallback exists for, so the fallback was unreachable in the one
situation that needed it. Both copies now go through _read_plugin_store, which
answers None for "not there" and for "there but unusable": a bad writable copy
falls through to the shipped default, and when neither is usable the load starts
from an empty store instead of aborting. Whenever the writable copy was not the
source, a usable one is written back so the next start does not fall back again.
_read_plugin_store also rejects a store whose "plugins" is missing or not a
mapping -- valid JSON is not the same as a valid store, and that shape is what
turned into the KeyError above.
Also drop the unused conf import in this module: the lint gate only inspects
changed files, so touching this file would otherwise turn the build red.
Covered by tests/test_plugin_store_atomic_write.py: a truncated store and one
that parsed without its "plugins" mapping must both load, a damaged writable
copy must fall back to the shipped default and be repaired, a save that dies
partway must leave the previous file byte-identical and no .tmp behind, and a
healthy store must load without being rewritten.
- Use write_plugin_config() instead of directly modifying plugin_config dict
- Add remove_plugin_config() to clear plugin config before reload
- Update plugins to use pconf() and write_plugin_config() for better config management