Repository navigation
feat(plugins): send connection context to DDL builders and add drop_table_sql - #1199
Merged
Merged
Conversation
Driver plugins can now adapt the host connection inputs and keep their own credentials in the OS keychain. - Manifest `connection_fields`: optional hidden/label/placeholder overrides for host, port, username, password and database. A hidden database input is no longer required, and hiding host also hides the Load databases action. Declared in manifest.schema.json and in the Tabularium extension schema. - `connection-modal.extra_fields` slot: optional `secretFields` (metadata only: value typed in this session, hasStoredValue, dirty) and `setSecretField(key, value)`; an empty value clears the secret. - Secrets are stored under `<id>:plugin:<driver>:<key>` and only their names are persisted (`plugin_secret_keys`). They are resolved into `params.extra[key]` in memory for every driver call, Test Connection, Load Databases and the MCP server, and never written to connections.json or returned to the frontend. - Lifecycle: save, update (driver switch drops the old secrets), duplicate, delete, group delete, export with credentials and backups, import (a metadata-only re-import keeps the stored secrets). - Disconnect and export no longer fail when a stored plugin secret cannot be read. - PLUGIN_GUIDE documents both contracts; @tabularis/plugin-api exposes the typed slot context and `PluginSecretFieldState`. Existing manifests, saved connections and UI extensions keep their behaviour: every new field is optional.
A plugin secret listed in `plugin_secret_keys` but missing from the OS keychain (synced connections file, keychain reset, downgrade) made every driver call of the connection fail, along with DDL, dump, MCP and duplicate. - Hydration is lenient everywhere: an invalid, missing or unreadable key (including a locked keychain or denied access) is logged by name, never by value, and left unset so the plugin reports the missing credential. Driver calls, Test Connection, Load Databases, export, backups and duplicate share one read path. - Duplicate copies only the values that can be read and lists only those keys on the copy. - Import skips invalid secret keys and oversized values with a warning instead of rejecting the payload, and writes plugin secrets to the keychain only after connections.json has been saved, so a failed save leaves no orphaned entries. - PLUGIN_GUIDE documents the unset-secret behaviour and what happens after a downgrade drops `plugin_secret_keys`.
Plugin SQL builders (create table, add/alter column, create index, foreign key, routine template) only received ConnectionParams when the plugin opted into connection metadata, and the commands ignored the database the user was browsing. Plugins with databases-as-projects could not qualify names correctly. Add DatabaseDriver::bind_connection_params so plugin drivers always get a snapshot with the resolved params (browsed database applied, multi-db selection coerced). The DDL commands accept an optional database override and the modals pass it. Built-in drivers keep the registered instance and saved params. The extra params field is additive.
Add an optional comment to ColumnDefinition (omitted from payloads when unset) and an opt-in column_comments capability. When a driver declares it, the add/modify column modal shows a description field and the DDL builder RPCs receive the comment; old_column carries the current value so plugins can detect changes. Built-in drivers keep column_comments false and their DDL unchanged.
Keyless row identity matched every comparable column with =. FLOAT64, FLOAT32 and BINARY_FLOAT/BINARY_DOUBLE were not recognised as approximate numerics, and STRUCT/ARRAY/MAP/RECORD/ROW/RANGE, array suffixes and document types (VARIANT, OBJECT) were compared through their serialized grid value, which either never matches or is rejected by the engine. The check is based on type names only, so it applies to every dialect.
The explorer always ran a generic DROP TABLE, which some engines reject for snapshot or external tables. Add an optional get_drop_table_sql RPC (params, table, schema, database) and a matching command. The explorer uses the plugin statement when one is returned and falls back to the generic, dialect-quoted DROP TABLE on -32601, null or for built-in drivers.
Plugins that answer unknown methods with a generic error or an empty array could no longer drop tables, because the explorer always asked get_drop_table_sql. The RPC is now opt-in through the manifest capability drop_table_sql (default false): without it the explorer runs the generic DROP TABLE without any RPC or credential resolution. A non-string result from an opted-in plugin is ignored with a warning.
PostgreSQL reports int[]/text[] columns as ARRAY (or type[]) and compares them with =, so excluding them weakened the identity of keyless tables and let UPDATE/DELETE match several rows. Only the parameterized and nested forms (ARRAY<...>, RANGE<...>, ROW(...), STRUCT, RECORD, REPEATED, MAP, TUPLE, NESTED) and document types stay excluded; PostgreSQL ranges such as int4range remain comparable.
Plugin DDL previews, routine templates and drop statements resolved SSH/SSM/K8s tunnels and keychain credentials on every call, so opening Create Table on a closed SSH connection could build a tunnel and previews failed offline. The SQL-builder commands now bind the saved params (connection id and browsed database applied, host/port untouched) without touching any tunnel or metadata discovery, and read keychain secrets leniently: a keychain failure binds the params without the secret.
The SQL-builder lookup bound the keychain password and connection URI but not the plugin-owned secrets, so builders that call the plugin's backend (for example get_drop_table_sql looking up the table kind) got no credential. Resolve them into params.extra with the tolerant plugin_secrets hydration: a missing or unreadable secret is logged and left unset, and tunnels are still never resolved.
This was referenced Oct 18, 2026
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| > | ||
| {t("modifyColumn.comment")} | ||
| </label> | ||
| <input autoCorrect="off" autoComplete="off" |
Contributor
There was a problem hiding this comment.
SUGGESTION: Description input is missing the text-input attributes its sibling inputs set
Every other free-text <input> in this modal (name, length, default) sets autoCapitalize="off" and spellCheck={false} alongside autoCorrect/autoComplete. This new one omits them, so the WebView may capitalise or spell-check the description. A column description is prose, so spell-check may be intentional; if so, ignore this. Otherwise align it with the siblings.
Suggested change
| <input autoCorrect="off" autoComplete="off" | |
| <input autoCorrect="off" autoCapitalize="off" autoComplete="off" spellCheck={false} |
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Collaborator
Author
There was a problem hiding this comment.
Fixed in 96b9475: the description input now sets autoCapitalize="off" and spellCheck={false} like the other inputs in the modal.
Contributor
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Notes
Files Reviewed (41 files)
Previous Review Summary (commit 86ef73d)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 86ef73d)Status: 1 Issue Found | Recommendation: Merge (optional nit) Overview
Issue Details (click to expand)SUGGESTION
Notes
Fix these issues in Kilo Cloud Files Reviewed (41 files)
Reviewed by claude-sonnet-5.5 · Input: 246 · Output: 2.3K · Cached: 193.9K Review guidance: REVIEW.md from base branch |
# Conflicts: # src-tauri/src/commands.rs
Contributor
Preview buildThe preview build of commit |
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.
Summary
Plugin drivers that model more than one database (for example projects holding datasets) generated DDL against the wrong database, because the DDL builder RPCs received
paramsonly with theconnection_metadataopt-in. This PR gives plugin DDL builders the context they need and lets plugins provide the statements the host cannot build generically.Changes
get_create_table_sql,get_add_column_sql,get_alter_column_sql,get_create_index_sql,get_create_foreign_key_sqlandroutine_create_templatealways receiveparamswith the browsed database applied. The commands take an optionaldatabase, passed by the Create Table, Modify Column, Create Index and Create FK modals.ColumnDefinition.commentand a new optional capabilitycolumn_comments. The Modify Column modal shows a Description input only when the driver declares it. i18n keys added to all locales. Built-in drivers are unchanged.get_drop_table_sql({ params, table, schema, database }→ string ornull), gated by a new optional capabilitydrop_table_sqlliketable_query_templates. Without the capability the explorer runs the genericDROP TABLEwith no RPC and no credential read.null, a blank string,-32601or any non-string result falls back to the generic statement.float32/float64/binary_float/binary_doublecount as approximate numerics, and nested types (STRUCT,RECORD,REPEATED,MAP,TUPLE,NESTED,ARRAY<...>,RANGE<...>,ROW(...),VARIANT,OBJECT,SUPER) are excluded. BareARRAY,type[]and PostgreSQL ranges stay comparable.PLUGIN_GUIDE.md: the DDL section is rewritten (the documented request shapes were wrong) and documents the new RPC and capabilities.Compatibility
get_drop_table_sql, and the extraparamson DDL requests is additive; every local plugin (duckdb, sqlserver, oracle, mongodb, libsql, elasticsearch, wordpress) was exercised over stdio with the new request shape.connection_metadatanow receive the saved host and port in DDL builder params instead of tunnel-resolved ones. Builders only generate SQL, so this only changes what they see.Tests
plugins/ddl_context_tests.rs,commands.rsbuilder tests (no tunnel resolution, secrets bound, keychain failures tolerated),dropTable.test.ts,rowIdentity.test.ts,driverCapabilities.test.ts.cargo test --all-targets,pnpm build,vitest,pnpm lintpass. Clippy reportstoo_many_argumentsonget_create_index_sql(8/7);get_create_foreign_key_sqlwas already flagged onmain.