Repository navigation
MCP robustness: validate/coerce inputs, report no-ops, GLOBAL vs LAYER tools, compact profile - #769
Merged
Merged
Conversation
…ools (#768) - sv_set_layer_settings / sv_update_layer refuse postfx and point to the LAYER effect tools. - coerceParams(): params/settings as object or JSON-string object; invalid JSON, arrays, null, empty and unsafe keys error. - coerceBool(): enabled accepts true/false and "true"/"false"; 0, "yes" error. - Mutating tools refuse calls whose arguments are all unknown. - withConfig reports changed true/false and skips no-op writes; drops stray layer settings.postfx (also in layers.normalizeLayer). - Read-after-write: applied values and visual in setter replies. - GLOBAL:/LAYER: description prefixes (TR GENEL:/KATMAN:) naming postfx vs layers[n].postfx. - Optional compact tool profile (mcp.toolProfile) with a toggle on the MCP card.
|
CaYaDraft — pull request Summary Files
Note Stopping here. I will not merge, push, or change this pull request until you ask.
|
- A write that fails or changes nothing while unknown arguments were sent returns 'Unknown argument(s): … <tool> takes: …' (generic, all write tools); tools with a patch argument add a 'put these fields inside patch' hint. - Stray layer settings.postfx is dropped when settings load in main (sanitizeLoaded); the layer ids are reported once as cleaned by the next MCP write (ctx.takeStartupCleaned) and logged. - Tests; test count 3182.
Settings are read twice at startup (startup parse, then the panel's get-settings via loadSettings) and the file keeps the stray key until the cleaned config is saved, so sanitizeLoaded logged the same ids twice. Track first sightings: cleanup still runs on every read; the log line and the cleaned report happen once per id. Test count 3183.
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
1 participant
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.
Closes #768
Summary
Every mutating MCP tool now either applies its input or returns a clear
{ ok:false, error }that names the right tool. Writes reportchanged: true/false. Effect tools say clearly whether they are GLOBAL or LAYER tools. An optional compact tool list helps small-context local models.Changes (per checklist)
sv_set_layer_settingsrefusespostfxin all three forms (settingsobject,settingsJSON string,key: "postfx").sv_update_layerrefusespatch.settings.postfx. The error namessv_add_layer_effect/sv_set_layer_effect_param.coerceParams(): used bysv_set_effect_param,sv_set_layer_effect_param,sv_add_effect,sv_add_layer_effect, and forsettingsinsv_set_layer_settings. It accepts an object or a JSON string that parses to an object.nulland non-objects are errors.*_paramtools, missing or empty params are errors.__proto__/constructor/prototypekeys at any depth are refused after parsing.coerceBool(): applies toenabledon the global/layer effect tools, the add tools andsv_set_modulation_enabled. It acceptstrue/falseand"true"/"false"(case-insensitive);0,"yes"andnullare errors.ignored, as before. Read-only tools are unchanged.changed:withConfigcompares the config before and after. A no-op answersok:true, changed:falseand is not saved or broadcast.sv_set_layer_settingsreturnsapplied(the stored values for the keys sent) andvisual. The*_paramtools returnapplied, and the effect enable tools now includevisual.GLOBAL:orLAYER:. It names its list (postfxvslayers[n].postfx), the matching tool of the other family, andsv_list_effects. The schema param descriptions name the target list too.mcp.toolProfilesetting (fulldefault, orcompact), with a "Compact tool list" toggle on the MCP card.tools/listthen returns 27 core read and common edit tools with one-sentence descriptions.layers[n].settings.postfxis dropped inlayers.normalizeLayer(panel), inmcp.normalizeLayer, and on any MCP write (reported ascleaned). It is not moved into the real chain, and the rest of the layer is untouched.Edge cases covered by tests
paramsas invalid JSON, a JSON array,null,{}or"{}".enabledas"false",0or"yes".Backward compatibility
changed,applied,visual).Tests
npm test: 3165 → 3178 (round 1) → 3182, all passing.tests/mcp-robustness-768.test.js(11 tests): one per finding, plus locked-layer order, unsafe keys, the helpers, andtools/listwith the compact profile.tests/mcp-coverage.test.js: GLOBAL/LAYER prefixes (EN+TR) and the compact profile.changed, GLOBAL/LAYER naming and the compact profile. README, README.tr and ROADMAP test counts are updated.Round 2
Unknown argument(s): … <tool> takes: …, with the original reason in parentheses. Before, it answered with "settings or key is required" plusignored. Example:sv_set_layer_settings {id, patch}.patch: on tools that takepatch, fields sent at the top level (sv_update_layer {id, locked:true}) get the hint "Put these fields inside patch".layers[].settings.postfxwhen settings load (sanitizeLoaded) and logs it. The next MCP write reports those layer ids once ascleaned.strayLayerSettingsPostfx. There is still no migration.