feat(tool): persist user enable/disable in flocks.json tool_settings#141
Merged
feat(tool): persist user enable/disable in flocks.json tool_settings#141
Conversation
The Tool UI toggle used to write directly back into the YAML plugin
file (<project>/.flocks/plugins/tools/**/*.yaml). Those YAMLs are
tracked by git and overwritten on upgrade, so every user click produced
a dirty working tree, a potential merge conflict on `git pull`, and a
lost customisation after each release. The same path also mutated
user-level YAML under ~/.flocks/plugins/tools, leaving users without a
single place to inspect their customisations.
Treat the YAML as the factory default and layer user choices into a
new flocks.json section that mirrors the existing `model_settings`:
"tool_settings": {
"onesec_dns": { "enabled": false }
}
Implementation:
- ConfigWriter gains list/get/set/delete_tool_setting CRUD; the whole
`tool_settings` key is popped when its last entry is removed so
flocks.json doesn't accumulate empty containers.
- ToolRegistry snapshots every tool's registration-time `enabled` flag
into `_enabled_defaults` (using setdefault so a plugin-refresh cycle
does not capture the overlay-mutated value as the new default) and
runs `_apply_tool_settings` after `_sync_api_service_states` on every
load.
- Service gate: an overlay can never *open* a tool whose backing API
service is disabled. The intent is still persisted so re-enabling
the service later restores it automatically; disabling is always
honoured.
- PATCH /api/tools/{name} writes the overlay and auto-deletes it when
the request equals the factory default. The response now carries
`enabled_default` and `enabled_customized` so the UI can render the
appropriate affordances.
- POST /api/tools/{name}/reset drops the overlay and restores the
factory default (still gated by service state).
- Web UI: tool detail drawer and local-tools list show a "Customized"
badge and a "Reset to default" action backed by the new endpoint.
The overlay applies uniformly to project-level YAML tools, user-level
YAML tools under ~/.flocks/plugins/tools, and non-YAML built-in /
plugin_py tools.
Tests: new unit and end-to-end coverage for ConfigWriter, the registry
overlay/service-gate/snapshot semantics, and the HTTP routes
(tests/config/test_config_writer.py, tests/tool/test_apply_tool_settings.py,
tests/server/test_tool_setting_routes.py — 65 tests).
NOTE: if your ~/.flocks/config/flocks.json has a previous experimental
`tool_overrides` key, rename it to `tool_settings` once. This feature
has not shipped to any release, so no auto-migration logic is added.
Made-with: Cursor
Review on PR #141 flagged two bugs in the factory-default snapshot: P1 (High) — refresh_plugin_tools left stale defaults behind. `_snapshot_enabled_defaults` used `setdefault` while `_unregister_plugin_tools` didn't touch `_enabled_defaults`, so a YAML edit that flipped `enabled:` from true to false was invisible to the registry on the next refresh cycle — `reset_tool_setting` would restore the old value and `PATCH` would auto-delete the overlay at the wrong threshold. P2 (Medium) — the single-tool reload paths never refreshed the snapshot either. `PUT /api/tools/{name}` and `POST /api/tools/{name}/reload` both end at `ToolRegistry.register`, which only replaced `_tools` and left the default snapshot untouched. So `enabled_default` served right after a reload echoed the first value ever observed, not the current YAML contents. Fix moves the snapshot lifecycle onto the same hook as `_tools`: - `register(tool)` writes `_enabled_defaults[name] = tool.info.enabled` unconditionally, so every reload (including same-name re-registration) refreshes the factory default. The write happens right after `apply_tool_catalog_defaults` but BEFORE `_sync_api_service_states` / `_apply_tool_settings` can mutate `info.enabled`, so overlays cannot leak into the snapshot. - `_unregister_plugin_tools` now pops the matching snapshot entry so a stale default cannot survive into the next refresh cycle. - `_snapshot_enabled_defaults` is kept as an idempotent safety net for code paths that bypass `register` (mostly tests) and switches from `setdefault` to direct assignment so stale entries self-heal. Tests: - `tests/tool/test_apply_tool_settings.py` gains four regressions: register refreshes the snapshot, overlay mutation never leaks into the snapshot, `_unregister_plugin_tools` pops the entry, and a full unregister/register "YAML upgrade" cycle picks up the new default. Also replaces the old `test_snapshot_preserves_builtin_default_on_refresh` (which encoded the now-broken setdefault semantics) with a safety-net assertion that `_snapshot_enabled_defaults` itself uses assignment. - `tests/server/test_tool_setting_routes.py` gains one HTTP-level regression that re-registers a tool and asserts `enabled_default` exposed over GET updates accordingly. Made-with: Cursor
Second-round review on the _enabled_defaults lifecycle flagged two issues introduced/left behind by the previous fix: 1. `_snapshot_enabled_defaults` switching from `setdefault` to direct assignment was a regression. It runs at the end of every `_load_plugin_tools` cycle, including every `refresh_plugin_tools`. By the second cycle, built-in / previously-registered tools whose provider is disabled already have `info.enabled = False` courtesy of the prior run's `_sync_api_service_states`, so direct assignment captures that post-sync value as the "factory default" — breaking `enabled_default` reporting and `reset_tool_setting` for any such tool. Since `register()` is already the authoritative writer (and `_unregister_plugin_tools` / `_unregister_dynamic_tools` pop to keep it fresh), the safety net only needs to fill genuine gaps. Reverted to `setdefault` and rewrote the docstring to spell out the contract. 2. `_unregister_dynamic_tools` never popped `_enabled_defaults`, so deleting a dynamic module left a stale factory default behind — exactly the asymmetry the prior patch fixed for plugin tools. Added the matching pop. Tests: - Replaced `test_snapshot_defaults_safety_net_uses_assignment` (which encoded the now-broken direct-assignment semantics) with `test_snapshot_safety_net_does_not_clobber_post_sync_state`, a direct reproduction of the refresh-after-sync regression path. - Added `test_snapshot_safety_net_fills_missing_entry` to pin the remaining intended behaviour (fill gaps, don't overwrite). - Added `test_unregister_dynamic_tools_drops_enabled_default` for the dynamic-tools symmetry fix. - Extended `isolated_registry` to shadow `_dynamic_tools_by_module` so the new test can't leak into later tests. Made-with: Cursor
xiami762
approved these changes
Apr 17, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The Tool UI toggle used to write directly back into the YAML plugin file (/.flocks/plugins/tools/**/*.yaml). Those YAMLs are tracked by git and overwritten on upgrade, so every user click produced a dirty working tree, a potential merge conflict on
git pull, and a lost customisation after each release. The same path also mutated user-level YAML under ~/.flocks/plugins/tools, leaving users without a single place to inspect their customisations.Treat the YAML as the factory default and layer user choices into a new flocks.json section that mirrors the existing
model_settings:Implementation:
tool_settingskey is popped when its last entry is removed so flocks.json doesn't accumulate empty containers.enabledflag into_enabled_defaults(using setdefault so a plugin-refresh cycle does not capture the overlay-mutated value as the new default) and runs_apply_tool_settingsafter_sync_api_service_stateson every load.enabled_defaultandenabled_customizedso the UI can render the appropriate affordances.The overlay applies uniformly to project-level YAML tools, user-level YAML tools under ~/.flocks/plugins/tools, and non-YAML built-in / plugin_py tools.