Skip to content

Commit b7a6838

Browse files
authored
feat(editor): the web editor's run decision as one shared function (#2204, part 1) (#2240)
* refactor(sql): ask one unreadable-run predicate in the gate and the safety dialog The gate asked whether a run never closes or a line comment holds a lone carriage return before it read a keyword, and the safety dialog asked the same two questions again to say why it asks. hasUnreadableRun in src/lib/sql/dangerous-query.ts is that predicate, and both now call it, so they cannot drift apart. No behaviour changes; tests/unit/sql/dangerous-query.test.ts is the gate module's own unit file. Part of #2204. * refactor(sql): move the VS Code child's nothing-to-run rule into runnableTarget query/target answered an empty statement when the run target held no code under the sql editor language, with a countCodeStatements check written in the child. runnableTarget in src/lib/sql/run-target.ts is runTarget with that rule, so the statement analysis can share it, and the child calls it. Every query/target answer stays as it was: the extension's query/target unit tests pass unchanged. Part of #2204. * refactor(editor): move the VS Code child's editorLanguageOf into tab-language The child read the editor language of a provider's text with a local editorLanguageOf over resolveTabType and editorLanguageForTabType. It now lives beside them in src/lib/editor/tab-language.ts, so the statement analysis reads the same answer, and connection/open and query/target call it there. Part of #2204. * refactor(editor): share the web editor's routing predicate and typed confirmation value Whether a run takes /api/db/multi-query was written inline in use-query-execution.ts, and the text a typed confirmation is compared with was a private function of QuerySafetyDialog. Both move to the new src/lib/editor/statement-analysis.ts as runsAsScript and typedConfirmationExpected, which hold no component, and the hook and the dialog call them there. The hook keeps every other routing condition and its "sql" default while the provider's metadata loads, so the route of every run is unchanged. Part of #2204. * feat(editor): analyzeStatement, the web editor's run decision in one function analyzeStatement in src/lib/editor/statement-analysis.ts takes the editor's text, selection and caret, the connection's type, capabilities and name, and answers nothing, refuse, run or confirm, with the exact statement to send, its range, its route, the unreadable notice and the typed value. It composes runnableTarget, statementRefusal, runsAsScript, isDangerousQuery, vocabularyTypedConfirmation, typedConfirmationExpected and hasUnreadableRun in the web editor's order, opens no connection, and is the decision the statement analysis route of #2204 will answer. Its test drives the 62 non-HTTP acceptance cases of #2204 on each engine's real capabilities, built without connecting, and a parity test holds each one to the web editor's own composition of the same parts. The one divergence, a target that holds no code, which the web sends and the analysis answers as nothing to run, is recorded as docs/BACKLOG.md U200. Part of #2204. * docs: name the shared run decision, and record the extension's refusal gap docs/VSCODE_EXTENSION.md names runnableTarget and editorLanguageOf as what the child calls, and section 8 says that Run in the extension does not apply the connection type's refusal, so a statement the web editor refuses before sending reaches query/run without a question and only the provider's guard stops it, recorded as docs/BACKLOG.md U201. docs/ARCHITECTURE.md says which functions analyzeStatement composes and which of them each caller uses. Part of #2204. * docs: name runsAsScript where comments named the hook's removed dialectIsSql The routing condition use-query-execution.ts kept in a local dialectIsSql is now runsAsScript, so the comments in the hook and Redis provider tests and the paragraph in docs/providers/redis.md that named the local now name the function. No behaviour changes. Part of #2204. * test(editor): hold the statement analysis parity test to what the web editor does The embedded store reads its text as SQL, so an unclosed literal there is unreadable; a new row pins that answer. A caret target that is an unclosed block comment holds no code, and the web asks about it before sending it, so the recorded divergence now admits that confirm, with two rows and U200 saying so. At a name-typed confirmation on a connection with no name, the web's dialog keeps Proceed disabled rather than refusing, and the test now composes that state instead of copying the refusal.
1 parent 3d84867 commit b7a6838

17 files changed

Lines changed: 1052 additions & 97 deletions

‎docs/ARCHITECTURE.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -291,6 +291,8 @@ See [LAUNCH.md](LAUNCH.md).
291291

292292
Multi-statement queries execute sequentially via `POST /api/db/multi-query`, one request per execution unit (`splitExecutionUnits`): a statement, or on SQL Server the whole batch between `GO` lines.
293293

294+
The run target, the connection type's refusal, the confirmation and the route of a run are decided by the functions `analyzeStatement` in `src/lib/editor/statement-analysis.ts` composes in the web editor's order: the web editor calls them one by one, and the VS Code extension's child calls the run target, the editor language and the gate.
295+
294296
### 4.4. Storage Abstraction Layer
295297

296298
- **Write-through cache architecture**: localStorage (L1 cache) + optional server storage (L2 persistent)

‎docs/BACKLOG.md‎

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ None of it is a GitHub issue.
3131
- [Drivers and connections](#drivers-and-connections) — D1-D452, U17 · 333
3232
- [Value interpolation](#value-interpolation) — V1
3333
- [Row editing](#row-editing) — R1–R3 · 3
34-
- [Studio UI and query execution](#studio-ui-and-query-execution) — X2-X26, U2-U199 · 185
34+
- [Studio UI and query execution](#studio-ui-and-query-execution) — X2-X26, U2-U201 · 187
3535
- [Dependencies](#dependencies) — P1-P13 · 11
3636
- [Documentation](#documentation) — DOC3-DOC35 · 29
3737
- [Release pipeline](#release-pipeline) — REL1-REL9 · 9
@@ -6943,6 +6943,27 @@ Found 2026-11-04 while moving the cross-platform jobs' one-time test setup out o
69436943

69446944
**Done when:** the test refuses a cyclic plan well inside the default timeout on a busy runner, by a Bun release that detects the cycle on the indented path as fast as on the plain one, or by a fence that refuses a cyclic plan before it serialises it, with the test kept.
69456945

6946+
### U200. The web editor sends a run target that holds no code, which the extension and the statement analysis answer as nothing to run
6947+
6948+
With the caret on a statement of comments only (`-- note` after `SELECT 1;`, or `SELECT 1; -- note` with the caret at the line's end), or with only a comment selected, `getEffectiveQuery` in `src/components/QueryEditor.tsx` answers the comment and `executeQuery` in `src/hooks/use-query-execution.ts` sends it to the engine; a text of blanks only is sent the same way.
6949+
A target that is an unclosed block comment (`/* open` after `SELECT 1;`, with the caret at the end) is asked about first, with the notice that part of the statement could not be read, and then sent.
6950+
The VS Code extension's `query/target` and `analyzeStatement` in `src/lib/editor/statement-analysis.ts` answer each of these with nothing to run, through `runnableTarget` in `src/lib/sql/run-target.ts`.
6951+
`tests/unit/sql/run-target.test.ts` pins `runTarget`'s own answer for such text, and the parity test in `tests/unit/editor/statement-analysis.test.ts` holds the divergence case by case.
6952+
The web cannot adopt `runnableTarget` alone: when the effective query is empty, `executeQuery` falls back to the tab's whole text.
6953+
Found 2026-11-03 while designing the statement analysis for #2204.
6954+
6955+
**Done when:** the web answers a target that holds no code with Nothing to run, through `runnableTarget`, and no empty effective query falls back to the tab text.
6956+
6957+
### U201. The VS Code extension sends a statement the web editor refuses to `query/run` without a question
6958+
6959+
The web editor asks `statementRefusal` (`src/lib/db/destructive-commands.ts`) before the confirmation gate, and on the nine connection types whose vocabulary row declares a refusal or a console text bound a refused statement is never sent or written to history (`src/hooks/use-query-execution.ts`).
6960+
The extension's `query/target` in `vscode-extension/src/backend/handlers.ts` answers only the gate's verdict, and the host in `vscode-extension/src/host/editor/run-query.ts` sends a statement the gate does not flag to `query/run` without asking, where `checkEditorQuery` applies the console text bound but not the refusal, so only the provider's own guard stops it.
6961+
Checked on 2026-11-04 with the shared functions: `isDangerousQuery` answers false and `statementRefusal` refuses both a Redis `GET a` with `DEL b` on the line after it and `kubectl delete pod web-1`; the Redis provider then refuses the first with the same sentence in `executeRedisCommand`, and the Kubernetes console refuses the second in `readKubectlRun` before any request.
6962+
`analyzeStatement` in `src/lib/editor/statement-analysis.ts` answers both with `refuse`, but `query/target` has no field for a refusal and the host no way to show one.
6963+
Found 2026-11-03 while designing the statement analysis for #2204.
6964+
6965+
**Done when:** `query/target` answers the connection type's refusal, the host shows it and sends nothing, and a test drives Redis `GET a\nDEL b` and `kubectl delete pod web-1` through it.
6966+
69466967
## Dependencies
69476968

69486969
### P1. The desktop shell's `glib` advisory has no reachable fix while Tauri v2 targets GTK 3

‎docs/VSCODE_EXTENSION.md‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ The webview build swaps three imports for shims through `shimPlugin` in `scripts
8888
| Connection profiles and secrets | `src/host/connections/profile.ts` (`readProfiles`, `toEntry`, `toWireConnection`), `connection-store.ts` (`ConnectionStore`), `secret-keys.ts` (`secretKey`, `SECRET_PATHS`) | `src/lib/storage/secret-fields.ts` (`secretFieldValues`, `withoutSecretFields`, `withSecretFieldValues` and its field classification maps), `EXTERNAL_DATABASE_TYPES` in `src/lib/db/compatibility.ts` | The profile sits in the `libredb.connections` setting; every secret sits in SecretStorage |
8989
| Add Connection | `src/host/connections/add-connection-flow.ts` (`AddConnectionFlow`, a pure state machine), `add-connection-ui.ts` (`addConnection`), `field-prompts.ts` (`FIELD_PROMPTS`), `src/backend/engine-catalog.ts` (`engineCatalog`) | `CONNECTION_FORM`, `CONNECTION_PICKER_ORDER` and the `connectionField*` helpers in `src/lib/db/connection-form.ts`, `CONNECTION_FIELDS` and `PLACEABLE_FIELDS` in `src/lib/db/connection-fields.ts`, `CONNECTION_STRING` in `src/lib/db/connection-string`, `parseConnectionString` in `src/lib/connection-string-parser.ts`, `PASTE_FORMATS_TEXT` in `src/lib/connection-paste-formats.ts` (the catalogue's `pasteFormats`, so the host bundles no provider), `testConnection` in `src/lib/api/test-connection.ts`, `newLocalId` in `src/lib/ids.ts` | The flow names no engine: every per-engine fact comes from the `engines/list` catalogue |
9090
| Connections tree | `src/host/tree/connections-tree.ts` (`ConnectionsTree`), `nodes.ts`, `tree-items.ts`, `icons.ts` | The kind helpers in `src/lib/db/object-kinds.ts` (`containerDepth`, `enumerableKinds`, `findKind`, `kindGroup`, ...), and in the child the provider methods `listContainers`, `countObjects`, `listObjects`, `describeObject` | Levels, folders and labels come from the provider's capabilities, recorded in `OpenConnections` |
91-
| Editor binding, Run, Cancel, status bar | `src/host/editor/` (`BindingStore`, `RunRegistry`, `RunService`, `runQuery`, `newQuery`, `selectRows`, `cancelQuery`, `changeConnection`, `ConnectionStatusBar`, `ContextKeys`, `DocumentLifecycle`) | In the child: `runTarget` in `src/lib/sql/run-target.ts`, `isDangerousQuery` in `src/lib/sql/dangerous-query.ts`, `resolveSqlGrammar`, `countCodeStatements`, `generateTableQuery` in `src/lib/query-generators.ts`, `editorLanguageForTabType` in `src/lib/editor/tab-language.ts` | Bindings live in memory only, so a window reload forgets them |
91+
| Editor binding, Run, Cancel, status bar | `src/host/editor/` (`BindingStore`, `RunRegistry`, `RunService`, `runQuery`, `newQuery`, `selectRows`, `cancelQuery`, `changeConnection`, `ConnectionStatusBar`, `ContextKeys`, `DocumentLifecycle`) | In the child: `runnableTarget` in `src/lib/sql/run-target.ts`, `isDangerousQuery` in `src/lib/sql/dangerous-query.ts`, `resolveSqlGrammar`, `generateTableQuery` in `src/lib/query-generators.ts`, `editorLanguageOf` in `src/lib/editor/tab-language.ts` | Bindings live in memory only, so a window reload forgets them |
9292
| Backend handlers and registry | `src/backend/handlers.ts` (`createHandlers`, `registerHandlers`, `toResponseError`), `registry.ts` (`ConnectionRegistry`), `start.ts` (`startBackend`), `main.ts` | `getOrCreateProvider`, `removeProvider`, `clearProviderCache`, `enabledProviderTypes` in `src/lib/db/factory.ts`; `checkEditorQuery` and `runEditorQuery` in `src/lib/api/editor-query.ts`; `databaseErrorBody` and `internalErrorBody` in `src/lib/api/error-body.ts`; `supportsQueryCancel`, `scopeToCatalog`, `executionUiFor` | `query/run` re-does the steps of `POST /api/db/query` for one statement through the same functions |
9393
| Backend lifecycle | `src/host/backend-supervisor.ts` (`BackendSupervisor`), `fork-options.ts`, `channel.ts` (`ENABLED_PROVIDERS`) | None; `RESTART_DELAYS_MS` copies `desktop/src-tauri/src/backoff.rs` | |
9494
| JSON-RPC protocol | `src/protocol.ts` (`BackendApi`, `METHODS`, `PROTOCOL_VERSION`), `src/host/backend-client.ts` (`BackendClient`, `BackendError`) | Types only (`DatabaseConnection`, `ProviderCapabilities`, `EditorQueryBody`, `ApiErrorCode`, ...) | Every Studio import in `protocol.ts` is type-only, so it adds no provider code to the host |
@@ -195,7 +195,7 @@ Step by step, with the code that does it:
195195
1. `runQuery` in `src/host/editor/run-query.ts` finds the editor, reads its binding from `BindingStore` (or binds it through `pickConnection`), refuses a second run in the same document with `ALREADY_RUNNING`, and reads `libredb.query.rowLimit` (1 to 10000, default 500) with `readRowLimit`.
196196
2. `RunRegistry.prepare` registers a preparing run with its own cancellation token, so Cancel works even while the connection opens.
197197
3. `BackendClient.ensureOpen` returns the connection's `ProviderDescription`, opening it first when needed; where the capabilities declare catalog sessions, the person picks a catalog once per binding.
198-
4. `query/target` sends the whole text, the selection and the cursor; the child applies the web editor's run target rule (`runTarget`) with the engine's grammar, and an empty target becomes "Nothing to run.".
198+
4. `query/target` sends the whole text, the selection and the cursor; the child applies the web editor's run target rule through `runnableTarget`, with the engine's grammar and the editor language `editorLanguageOf` reads from its capabilities, and a target that holds no code becomes "Nothing to run.".
199199
The answer also says whether the web editor would ask before running that statement: the child reads it with `isDangerousQuery` under the connection's type, the gate behind the web editor's Query Safety dialog.
200200
The range that will run is highlighted for one second, and a statement the gate flags then asks in a modal `showWarningMessage` that shows it and offers Run Anyway; Cancel or Escape runs nothing.
201201
5. `RunService.execute` mints a fresh `queryId`, turns the preparing run into a running `RunEntry`, tells the presenter, and sends `query/run` inside a progress shown on the Results view.
@@ -370,6 +370,7 @@ Select Rows asks the child for `objects/selectStatement`, which writes the state
370370
- **No login or roles.** The extension serves one person's VS Code: no host or backend code signs anyone in or checks a role, and the host bundle check refuses `src/lib/config/auth-env.ts`.
371371
- **No storage layer or sync.** Connections are the setting plus SecretStorage, not `src/lib/storage`; the webview's `@/lib/storage` shim throws on any use; editor bindings are in memory only.
372372
- **A plainer dangerous-query confirmation.** The statement is read by the web editor's own `isDangerousQuery`, but the question is one modal button: no AI risk analysis, and no typed confirmation where the web editor asks for one (an etcd range delete or lease revoke).
373+
Nor does Run apply the connection type's refusal: a statement the web editor refuses before sending it (a Redis `GET a` with `DEL b` on the line after it, or `kubectl delete pod web-1`) reaches `query/run` without a question, and only the provider's own guard stops it (`docs/BACKLOG.md` U201).
373374
Select Rows and Load More ask nothing: the first runs the read the engine's generator built for the object (a SELECT, a find, a `get`, an `aws s3 ls`), and the second reads more pages of a statement already confirmed.
374375
That differs from the web editor on purpose, whose table click goes through the gate, so a generated read on an object whose name the gate cannot read safely (a ClickHouse table named `a$$b`) asks there and not here.
375376
- **No query events or governance.** The child bundle refuses `src/lib/governance/start.ts` and `src/lib/query-events/sink.ts`, and `query/run` runs no admission gate.

‎docs/providers/redis.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -644,7 +644,7 @@ So the next user's command does not fail, it silently does not happen, and the s
644644
**WHICH CALLERS END IT, AND WHY THAT CHANGED.** The surface now has TWO callers in the product,
645645
`POST /api/db/multi-query` and `POST /api/db/query`, each awaiting it in a `finally` under a call
646646
scope of its own. **The editor never sends a Redis buffer to the first of those.**
647-
`use-query-execution.ts` gates the multi-statement route on `dialectIsSql`, read from
647+
`use-query-execution.ts` gates the multi-statement route on `runsAsScript`, read from
648648
`queryLanguage`, and this provider declares `json` (§9), so an editor run goes to
649649
`POST /api/db/query` (§12.2).
650650

‎src/components/QuerySafetyDialog.tsx‎

Lines changed: 11 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,8 @@ import { useReturnFocus } from "@/hooks/use-return-focus";
1616
import { QUERY_SAFETY_ANALYSIS_TIMEOUT_MS } from "@/lib/llm/query-safety";
1717
import { cn } from "@/lib/utils";
1818
import { vocabularySendsToModel, vocabularyTypedConfirmation } from "@/lib/db/destructive-commands";
19-
import type { TypedConfirmationAsk } from "@/lib/db/types";
20-
import { readsSqlText, resolveSqlGrammar } from "@/lib/sql/grammar";
21-
import { hasLoneCarriageReturnInLineComment, hasUnterminatedSpan } from "@/lib/sql/spans";
19+
import { typedConfirmationExpected } from "@/lib/editor/statement-analysis";
20+
import { hasUnreadableRun } from "@/lib/sql/dangerous-query";
2221
import type { DatabaseType } from "@/lib/types";
2322

2423
// The gate lives in `@/lib/sql/dangerous-query` so the VS Code extension's child, which bundles no component, asks
@@ -87,16 +86,6 @@ const MISSING_CONNECTION_NAME =
8786
const LOCAL_CLASSIFICATION =
8887
"This editor checked the statement itself: statements for this engine are not sent to an AI provider for a risk analysis.";
8988

90-
/**
91-
* The text a typed ask is compared with: the ask's own text, or the connection's name, or undefined where the ask is
92-
* the name and there is none to type, which the dialog answers with MISSING_CONNECTION_NAME.
93-
*/
94-
function typedConfirmationText(ask: TypedConfirmationAsk, connectionName: string | undefined): string | undefined {
95-
if (ask.type === "text") return ask.text;
96-
// An empty name is no name: an exact comparison would let an empty field confirm it.
97-
return connectionName === "" ? undefined : connectionName;
98-
}
99-
10089
/**
10190
* Why the dialog stopped waiting for the AI analysis before it answered: the person chose to skip it, or it did not
10291
* finish within QUERY_SAFETY_ANALYSIS_TIMEOUT_MS. Either way Execute no longer waits on it.
@@ -191,25 +180,23 @@ export function QuerySafetyDialog({
191180
*
192181
* Memoised because the analysis stream re-renders this component per chunk while
193182
* the query text does not change, and the scan is linear in that text.
183+
*
184+
* The gate's own predicate, so the notice and the gate cannot disagree; it answers
185+
* false where the text is not SQL, which this notice must never call unreadable.
194186
*/
195-
const unreadableRun = useMemo(() => {
196-
const type = databaseType as DatabaseType | undefined;
197-
// Asked first: a SQL span reader's verdict about text that is not SQL is not
198-
// evidence of anything, and saying "could not be read" about a Mongo document
199-
// that reads fine is the false alarm this notice exists to avoid.
200-
const grammar = resolveSqlGrammar(type);
201-
return (
202-
readsSqlText(type) && (hasUnterminatedSpan(query, grammar) || hasLoneCarriageReturnInLineComment(query, grammar))
203-
);
204-
}, [query, databaseType]);
187+
const unreadableRun = useMemo(
188+
() => hasUnreadableRun(query, databaseType as DatabaseType | undefined),
189+
[query, databaseType],
190+
);
205191

206192
// What this engine's vocabulary asks the person to type before the statement runs, read from the table the gate
207193
// that opened this dialog read, so the two cannot disagree about a statement (#1089, section 5.5).
208194
const typedAsk = useMemo(
209195
() => vocabularyTypedConfirmation(databaseType as DatabaseType | undefined, query),
210196
[query, databaseType],
211197
);
212-
const typedExpected = typedAsk === undefined ? undefined : typedConfirmationText(typedAsk, connectionName);
198+
// Undefined where the ask is the connection's name and there is none to type, answered with MISSING_CONNECTION_NAME.
199+
const typedExpected = typedAsk === undefined ? undefined : typedConfirmationExpected(typedAsk, connectionName);
213200
// Proceed waits for the analysis and for an exact match, and a refusal in place of the field is never a match,
214201
// whatever was typed before it replaced the field. The wait for the analysis is bounded: Skip ends it at once and
215202
// QUERY_SAFETY_ANALYSIS_TIMEOUT_MS ends it anyway, so a slow or hung model can no longer hold every write back.

0 commit comments

Comments
 (0)