fix(mcp): resolve saved-but-inactive connections in bridge - #132
Merged
Conversation
MCP bridge resolve_connection only checked in-memory connections, so connections saved in .store.dat but not yet activated in the UI session failed with misleading errors. Reuse capabilities::sql::resolve_adapter, which reads the store in-process and connects — credentials never leave the app. McpPolicy authorization still runs upstream in invoke_with_policy. Also make get_connection_id errors actionable: distinguish 'no connection provided' (point to sqlkit__list_connections / Settings → MCP Bridge) from a malformed internal config. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
The handler always returned {"value": null} without reading the store —
dead code that misleads agents. MCP should not expose internal app store
data, so remove the capability and its registration.
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)
Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
sqlkit__execute_query was a single Elevated capability carrying all SQL — INSERT/UPDATE/DELETE/DDL ran with no Destructive gate, and parallel_ok blocked concurrent reads. Split by statement class so McpPolicy can gate each risk level: - sqlkit__execute_query: read-only (SELECT/SHOW/EXPLAIN) → Safe, parallel - sqlkit__execute_write: INSERT/UPDATE/MERGE → Elevated - sqlkit__execute_delete: DELETE/TRUNCATE → Destructive - sqlkit__execute_ddl: CREATE/ALTER/DROP → Destructive classify_sql parses with sqlparser (dialect-aware). execute_query now rejects write/delete/ddl statements with actionable guidance pointing to the split tools. New module sql_write.rs shares resolve_adapter and execute_on_adapter from sql.rs. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
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
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.
fix + feat: MCP connection resolution and SQL write tool split
What
Four commits on this branch:
fix(mcp): resolve saved-but-inactive connections in bridge —
mcp_bridge::resolve_connectionnow reusescapabilities::sql::resolve_adapter, which reads.store.datin-process and connects, instead of only checking in-memory active connections. Credentials never leave the app;McpPolicyauthorization still runs upstream ininvoke_with_policy. Error messages fromget_connection_idare now actionable.chore: sync Cargo.lock sqlkit version to 0.8.4 — lock was stale at 0.8.1 while Cargo.toml is 0.8.4.
fix(mcp): remove get_store_value stub capability — the handler always returned
{"value": null}without reading the store; MCP should not expose internal app store data.feat(mcp): split SQL write tools by risk level —
sqlkit__execute_querywas a single Elevated capability carrying all SQL (INSERT/UPDATE/DELETE/DDL ran with no Destructive gate). Split by statement class:sqlkit__execute_query: read-only (SELECT/SHOW/EXPLAIN) → Safe, parallel_ok=truesqlkit__execute_write: INSERT/UPDATE/MERGE → Elevatedsqlkit__execute_delete: DELETE/TRUNCATE → Destructive (Confirm Destructive gate)sqlkit__execute_ddl: CREATE/ALTER/DROP → Destructiveclassify_sqlparses with sqlparser (dialect-aware).execute_queryrejects write/delete/ddl with actionable guidance to the split tools. New modulesql_write.rssharesresolve_adapter/execute_on_adapterfromsql.rs.Why
Missing connectionId in connection config/Connection not found).get_store_valuewas a dead stub misleading agents.Checklist
cargo checkpassescargo test --libpasses (317 tests, incl. 10 new sql_write classification tests)