Raised out of the round-1 review of #222, which fixed this wording in skills/keepalive/SKILL.md only. The other three skills carry the same defect, and it is pre-existing on main rather than introduced by that PR, so it gets its own issue per the repo's rule about unrelated defects.
The defect
settings.py config prints an option_<name>= line only for keys actually present in pluginConfigs[…].options, and reports source=(none) only when the whole options object is missing or empty (scripts/settings.py:570-575).
All four skills document the fallback as though it were keyed on source=:
| File |
Line |
Text |
skills/status/SKILL.md |
23 |
Use its `option_port=`, or 8787 if it reports `source=(none)`. |
skills/uninstall/SKILL.md |
27 |
Use its `option_port=`, or 8787 if it reports `source=(none)`. |
skills/install/SKILL.md |
108-109 |
option_port=… / option_preset=… — use these + source=(none) — nothing configured; the plugin.json defaults apply |
So a user with a partial config — the common case, e.g. --config preset=house with the port left alone — gets a real source= and no option_port= line. The documented fallback does not apply, because source= is not (none), and there is no value to use either. The model is left to invent one at exactly the point these sections warn is dangerous.
For uninstall that is the worse outcome: it builds the removal URL from the port, so an invented port matches nothing and the removal silently finds nothing to remove — the failure mode #221 was about, reached by a different route.
Fix
Make the fallback per-option rather than keyed on source=, as #222 now does for keepalive:
Any option the output does not list is unconfigured — use the plugin.json default for that one (port 8787, preset cache), whatever source= says.
Worth considering a test that asserts the partial-config shape, since all four instances were written from the same incorrect mental model and a fifth skill would inherit it. TestTheConfiguredPortCanActuallyBeHonoured currently only exercises the both-keys-present case.
Raised out of the round-1 review of #222, which fixed this wording in
skills/keepalive/SKILL.mdonly. The other three skills carry the same defect, and it is pre-existing onmainrather than introduced by that PR, so it gets its own issue per the repo's rule about unrelated defects.The defect
settings.py configprints anoption_<name>=line only for keys actually present inpluginConfigs[…].options, and reportssource=(none)only when the whole options object is missing or empty (scripts/settings.py:570-575).All four skills document the fallback as though it were keyed on
source=:skills/status/SKILL.mdUse its `option_port=`, or 8787 if it reports `source=(none)`.skills/uninstall/SKILL.mdUse its `option_port=`, or 8787 if it reports `source=(none)`.skills/install/SKILL.mdoption_port=… / option_preset=… — use these+source=(none) — nothing configured; the plugin.json defaults applySo a user with a partial config — the common case, e.g.
--config preset=housewith the port left alone — gets a realsource=and nooption_port=line. The documented fallback does not apply, becausesource=is not(none), and there is no value to use either. The model is left to invent one at exactly the point these sections warn is dangerous.For
uninstallthat is the worse outcome: it builds the removal URL from the port, so an invented port matches nothing and the removal silently finds nothing to remove — the failure mode #221 was about, reached by a different route.Fix
Make the fallback per-option rather than keyed on
source=, as #222 now does forkeepalive:Worth considering a test that asserts the partial-config shape, since all four instances were written from the same incorrect mental model and a fifth skill would inherit it.
TestTheConfiguredPortCanActuallyBeHonouredcurrently only exercises the both-keys-present case.