feat: support plugin-owned connection fields and secrets - #629
Conversation
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
|
|
||
| if !conn.plugin_secret_keys.is_empty() { | ||
| let cache = credential_cache::CredentialCache::default(); | ||
| crate::plugin_secrets::hydrate_connection(&cache, &mut conn).map_err(|message| { |
There was a problem hiding this comment.
WARNING: Plugin secret hydration runs blocking keychain I/O on the async runtime thread.
The adjacent DB-password (line 255) and connection-URI (line 286) reads in this same function both wrap their keychain access in tokio::task::spawn_blocking. hydrate_connection → get_plugin_secret_cached → keychain_utils::get_plugin_secret performs synchronous keychain reads directly on the async worker thread, which can stall the MCP server's runtime — notably on platforms where keychain access is slow or requires an unlock prompt. Mirror the surrounding pattern and move the hydration into spawn_blocking (one read per secret key, like the URI path does).
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Code Review SummaryStatus: 1 Issue Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Files Reviewed (38 files)
Fix these issues in Kilo Cloud Reviewed by glm-5.2 · Input: 72.1K · Output: 20.6K · Cached: 772.7K |
1 participant
Summary
Compatibility
All new manifest, model, request, and UI context fields are optional or defaulted. Existing manifests, saved connections, UI extensions, and Password-based plugins keep their current behavior.
Stored plugin secrets are never returned to the frontend as plaintext. Existing values are represented only by
hasStoredValue, and an unchanged field remains untouched.Tabularium prerequisite
Tabularium must be updated to accept
connection_fieldsin the driver manifest schema before plugins using this property can be published. The current schema served byregistry.tabularis.devrejects it as an additional property.This PR updates the host-side schema and documentation, but it does not update the operational Tabularium registry schema. That registry change must be deployed before publishing the dependent BigQuery plugin PR.
Dependent plugin PR: TabularisDB/tabularis-bigquery-plugin#2
Validation
cargo test --all-targets: 1137 passed, 4 ignored; integration suite 9 ignoredcargo test --all-targets --no-runpnpm exec vitest run: 220 files, 3715 tests passedpnpm buildpnpm lintpnpm exec tsc --noEmitgit diff --checkcargo clippy --all-targets -- -D warningsstill reports existing workspace-wide lint debt unrelated to this change. The new code does not add the MSRV incompatibility found during review.