From a9438eccdea3710ada60ffcdab9482130d12bf87 Mon Sep 17 00:00:00 2001 From: Keenan Simpson Date: Mon, 27 Jul 2026 16:57:32 +0200 Subject: [PATCH 1/4] Add CLI Tools settings UI, fix stuck cancel, DO-block limit clause, and Windows CLI/session bugs - #39: Settings -> CLI Tools management UI (status, cached versions with disk usage, remove-cached, pre-download picker) backed by new cli_remove_cached command - #212: Cancel Query left the app unable to run further queries; the abandoned run's finally block could still clobber isExecuting/generation state after cancellation - applyQueryLimit / query executor no longer appends a LIMIT clause to PL/pgSQL DO $$ ... $$ blocks containing RETURNING - Double-clicking a table-nested trigger no longer mis-resolves its DDL lookup path - Saved query tabs are no longer wiped from sessions.json by a save/restore race on launch - MySQL CLI auto-download now works on Windows (missing URL + binary relocation for .exe-suffixed archives, shared with mongosh) - CLI subprocess calls no longer flash a console window on Windows - Saved queries no longer silently disappear on Windows due to unstable machine-ID detection; machine ID is now cached in the OS keyring and fingerprint/decrypt failures are now logged instead of swallowed --- CHANGELOG.md | 7 + src-tauri/src/cli.rs | 106 ++++++++-- src-tauri/src/lib.rs | 1 + src-tauri/src/storage.rs | 25 ++- src/components/editor/QueryEditor.tsx | 31 +-- src/components/explorer/DatabaseExplorer.tsx | 25 ++- src/components/layout/MainContent.tsx | 45 ++++- src/components/settings/SettingsDialog.tsx | 194 ++++++++++++++++++- src/lib/ipc.ts | 5 + src/store/cliStore.ts | 9 + src/store/savedQueryStore.ts | 3 +- src/utils/applyQueryLimit.test.ts | 30 +++ src/utils/applyQueryLimit.ts | 5 + 13 files changed, 421 insertions(+), 65 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d000d18..95c47a9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,12 @@ All notable changes to QueryDen are documented here. This project adheres to [Se - **[#15](https://github.com/openidle-dev/queryden/issues/15) — Argon2id parameters are now explicitly locked.** Replaced `Argon2::default()` with an explicit `Params::new(19456, 2, 1, None)` — 19 MiB memory cost, 2 iterations, 1 parallel thread, using Argon2id v1.3. These match the actual defaults of the argon2 0.5 crate and prevent silent parameter drift across crate version bumps. ### Fixed +- **Double-clicking a table-nested trigger showed "Trigger schema.table-triggerName not found."** The double-click DDL handler reconstructed a `schema.name` path from the tree node's id for every leaf type, but table-scoped trigger nodes have an id of the form `trig-schema.table-triggerName` — one dash-joined segment more than the reconstruction logic expected — so the table name got folded into the trigger name. `pg_get_triggerdef` looks up triggers by plain `tgname` only, so triggers now skip the path reconstruction and use the node's plain name. +- **Saved query tabs stopped being restored on app launch.** The "save session" effect fired on mount with the initial empty tab list and could win a race against the async session-restore effect (which waits on `initialLoadDone` plus an IPC round-trip), overwriting `sessions.json` with an empty tab list before it was ever read back. The save effect now waits until the restore attempt has completed before persisting. +- **MySQL CLI auto-download did not work on Windows.** `download_spec` had no Windows URL for MySQL, so `cli_ensure`/`cli_download_version` always fell back to the system-install hint on Windows even when auto-download was expected. Added the Windows `mysql-9.0.0-winx64.zip` archive URL alongside the existing macOS ones. +- **Downloaded CLI archives (mongosh, and now MySQL) silently failed to install on Windows.** `relocate_binaries` searched for unsuffixed binary names (`mongosh`, `mysql`) inside the extracted archive, but Windows archives contain `mongosh.exe`/`mysql.exe`, so the relocation step found nothing and the post-download verification reported "Binary not found after download." Fixed to search/move platform-suffixed binary names, with a fallback for archives that extract straight into `bin/` without a nested versioned subdirectory. +- **CLI subprocess calls flashed a console window on Windows.** `cli_get_version`, `cli_detect_pg_version`, `cli_test_connection`, `cli_execute_query`, and `cli_list_databases` all spawned `psql`/`mysql`/etc. without `CREATE_NO_WINDOW`, so each query or connection test briefly popped a console window. All five spawn sites now apply the same no-window flag already used elsewhere in the codebase for PowerShell calls. +- **Saved queries could silently disappear on Windows.** Windows machine-ID detection shells out to PowerShell/WMI (`Get-CimInstance`), which can be flaky between app launches (AV/EDR interference, WMI not yet ready on a fresh boot). A different result on a later launch silently rotated the machine-locked encryption key, and `load_saved_queries` treated the resulting decrypt/fingerprint failure as "no saved queries" with zero diagnostics. The first successfully-detected machine ID is now persisted to the OS keyring (matching the existing master-key persistence pattern) so it stays stable across launches, and fingerprint-mismatch/decrypt-failure paths now log a warning instead of failing silently. The frontend's `loadFromFile` also had a bare `catch {}` that swallowed the error with no log — it now logs via `logger.error`, matching `saveToFile`. - **[#217](https://github.com/openidle-dev/queryden/issues/217) — Running a query now gives instant feedback and can't be triggered twice.** Pressing `Ctrl+Enter` previously showed no "running" state until *after* the database connection handshake (`Database.load`), which can take seconds on a cold connection — so the editor looked idle and a second press would run the query again. `setIsExecuting(true)` now fires synchronously at the start of execution (the spinner and tab status glyph appear the instant you press the shortcut), and a re-entrancy guard drops a duplicate trigger while a run is already in flight. - **You can now close the last remaining query tab.** The tab close button was hidden whenever only one tab was open, leaving no way to close the final editor. The button is always available now; closing the last tab falls back to the empty-state launcher. - **PSQL Console error messages no longer flash and disappear.** Errors in the CLI execution path (PostgreSQL version unknown, download cancelled, download failed, psql not found, `\watch` errors) are now committed as persistent `psqlConsoleEntry` entries before the live-output grace period expires, preventing the output from going blank after 300ms. The `\watch` loop-end path also commits accumulated output. Fixes Windows WebView2 race where `showLiveGrace` expired without entries. @@ -54,6 +60,7 @@ All notable changes to QueryDen are documented here. This project adheres to [Se - **PK detection now prefers real database metadata over name heuristics.** `loadTableDetails` queries actual PK columns from `information_schema.table_constraints` (PostgreSQL/MySQL) or `PRAGMA table_info` (SQLite). The post-SELECT FK auto-load path also detects PKs in parallel. All CRUD operations use a unified `getPrimaryKey` helper that checks `tableSchema.primaryKeys` first, then falls back to expanded name heuristics (`id`, `uuid`, `uid`, `pk`, `row_id`, `object_id`, `key`, `code`, `_id`, `{table}_id`). ### Added +- **[#39](https://github.com/openidle-dev/queryden/issues/39) — Settings → CLI Tools management UI.** A new settings section lists PostgreSQL/MySQL/MongoDB/Redis CLI status (system PATH detection), cached downloaded versions with disk usage, a remove button per cached version (new `cli_remove_cached` command), and a version picker to pre-download a tool before you need it — avoiding the wait for an ~80MB download mid-query. The existing inline download-on-demand flow during query execution remains as a fallback for anyone who skips this UI. - **[#216](https://github.com/openidle-dev/queryden/issues/216) — The query editor now highlights the statement under the cursor (DataGrip-style "will run" block).** A subtle accent-tinted band (plus a thin accent bar in the line-number margin) marks the statement that `Ctrl+Enter` will execute, updating as the caret moves. It is suppressed when you have an active selection (the selection itself shows what runs) and when the whole buffer is a single statement. Run-at-cursor and the highlight share one resolver (`resolveStatementAtOffset`), so the highlight can never disagree with what actually runs. - **FK-aware inline navigation in the results grid.** FK columns render as styled link cells with an external-link icon. Clicking the icon fires a `SELECT * FROM refTable WHERE refCol = value` query, navigating directly to the referenced row — enabling quick parent-row lookup from any query result. - **Schema-aware Add Row modal.** The Add Row dialog now reads real column metadata (name, type, nullable, default) from the table schema. NOT NULL columns without a default are validated before save with inline error badges. FK columns render as searchable dropdowns (`loadFKOptions` queries the referenced table with ILIKE/LIKE search across display columns, returning up to 50 matches). Default values from column definitions are pre-filled (stripping type casts and surrounding quotes). diff --git a/src-tauri/src/cli.rs b/src-tauri/src/cli.rs index 4bf5671..716e859 100644 --- a/src-tauri/src/cli.rs +++ b/src-tauri/src/cli.rs @@ -33,6 +33,20 @@ use tauri::{AppHandle, Emitter, Manager, State}; use tokio::io::{AsyncBufReadExt, BufReader}; use tokio::process::Command; +#[cfg(target_os = "windows")] +const CREATE_NO_WINDOW: u32 = 0x08000000; + +/// Suppress the flashing console window Windows spawns for each subprocess. +/// No-op on other platforms. +fn no_console_window(cmd: &mut Command) -> &mut Command { + #[cfg(target_os = "windows")] + { + use tokio::process::CommandExt; + cmd.creation_flags(CREATE_NO_WINDOW); + } + cmd +} + // ─── Tool kind ─────────────────────────────────────────────────────────────── #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] @@ -148,11 +162,13 @@ fn download_spec(kind: ToolKind, _major_version: Option) -> Option { - // MySQL compressed tarballs on dev.mysql.com + // MySQL compressed archives on dev.mysql.com let url = if cfg!(all(target_os = "macos", target_arch = "aarch64")) { "https://dev.mysql.com/get/Downloads/mysql-9.0.0-macos14-arm64.tar.gz" } else if cfg!(all(target_os = "macos", target_arch = "x86_64")) { "https://dev.mysql.com/get/Downloads/mysql-9.0.0-macos14-x86_64.tar.gz" + } else if cfg!(target_os = "windows") { + "https://dev.mysql.com/get/Downloads/mysql-9.0.0-winx64.zip" } else { return None; }; @@ -360,6 +376,8 @@ impl CliManager { let bin_dir = extracted_dir.join("bin"); std::fs::create_dir_all(&bin_dir).map_err(|e| e.to_string())?; + let ext = if cfg!(target_os = "windows") { ".exe" } else { "" }; + // Scan extracted_dir for the actual binaries (they're in a versioned subdir) let entries = std::fs::read_dir(extracted_dir).map_err(|e| e.to_string())?; let top_level_dirs: Vec = entries @@ -369,17 +387,22 @@ impl CliManager { .collect(); // Find the subdirectory that contains our binaries - // PostgreSQL: postgresql-16.8/bin/psql exists + // PostgreSQL: postgresql-16.8/bin/psql(.exe) exists let subdir = top_level_dirs.iter().find(|d| { let bin = d.join("bin"); - bin.join(kind.primary_binary()).exists() + bin.join(format!("{}{}", kind.primary_binary(), ext)).exists() }); - if let Some(src_dir) = subdir { - for binary in kind.all_binaries() { - let src = src_dir.join("bin").join(binary); - if src.exists() { - let dst = bin_dir.join(binary); + // Some archives (e.g. mongosh, mysql on Windows) extract binaries straight + // into extracted_dir/bin rather than a nested versioned subdir. + let search_dir = subdir.cloned().unwrap_or_else(|| extracted_dir.clone()); + + for binary in kind.all_binaries() { + let name = format!("{}{}", binary, ext); + let src = search_dir.join("bin").join(&name); + if src.exists() { + let dst = bin_dir.join(&name); + if dst != src { if dst.exists() { std::fs::remove_file(&dst).ok(); } @@ -465,6 +488,33 @@ pub struct CachedTool { pub major_version: u32, pub binaries: Vec, pub path: String, + pub size_bytes: u64, +} + +/// Recursively sum file sizes under `path`. +fn dir_size(path: &std::path::Path) -> u64 { + let mut total = 0u64; + if let Ok(entries) = std::fs::read_dir(path) { + for entry in entries.flatten() { + let p = entry.path(); + if p.is_dir() { + total += dir_size(&p); + } else if let Ok(meta) = entry.metadata() { + total += meta.len(); + } + } + } + total +} + +fn parse_tool_kind(tool_kind: &str) -> Result { + match tool_kind { + "postgresql" => Ok(ToolKind::Psql), + "mysql" => Ok(ToolKind::MySql), + "mongodb" => Ok(ToolKind::Mongo), + "redis" => Ok(ToolKind::Redis), + _ => Err(format!("Unknown tool: {}", tool_kind)), + } } /// Check all tools — system and bundled — and return their status. @@ -541,12 +591,29 @@ pub async fn cli_list_cached( major_version, binaries, path: entry.path().to_string_lossy().to_string(), + size_bytes: dir_size(&entry.path()), }); } Ok(cached) } +/// Remove a cached (downloaded) CLI tool version from disk to free space. +/// Does not touch system-installed binaries — only the versioned cache dir. +#[tauri::command] +pub async fn cli_remove_cached( + tool_kind: String, + major_version: u32, + manager: State<'_, CliManager>, +) -> Result<(), String> { + let kind = parse_tool_kind(&tool_kind)?; + let dir = manager.versioned_dir(kind, major_version); + if !dir.exists() { + return Ok(()); + } + std::fs::remove_dir_all(&dir).map_err(|e| format!("Failed to remove cached tool: {}", e)) +} + /// Download a specific version of a CLI tool. #[tauri::command] pub async fn cli_download_version( @@ -734,11 +801,10 @@ pub async fn cli_get_version( ToolKind::Mongo | ToolKind::Redis => "--version", }; - let output = Command::new(&binary) - .arg(flag) - .output() - .await - .map_err(|e| e.to_string())?; + let mut cmd = Command::new(&binary); + cmd.arg(flag); + no_console_window(&mut cmd); + let output = cmd.output().await.map_err(|e| e.to_string())?; if !output.status.success() { return Err("Version check failed".to_string()); @@ -765,8 +831,8 @@ pub async fn cli_detect_pg_version( .await .ok_or_else(|| ToolKind::Psql.system_install_hint().to_string())?; - let output = Command::new(&binary) - .env("PGPASSWORD", &password) + let mut cmd = Command::new(&binary); + cmd.env("PGPASSWORD", &password) .arg("-h") .arg(&host) .arg("-p") @@ -779,10 +845,9 @@ pub async fn cli_detect_pg_version( .arg("-A") // unaligned .arg("-w") // never prompt .arg("-c") - .arg("SELECT version()") - .output() - .await - .map_err(|e| e.to_string())?; + .arg("SELECT version()"); + no_console_window(&mut cmd); + let output = cmd.output().await.map_err(|e| e.to_string())?; if !output.status.success() { let stderr = String::from_utf8_lossy(&output.stderr); @@ -876,6 +941,7 @@ pub async fn cli_test_connection( } } + no_console_window(&mut cmd); let output = cmd.output().await.map_err(|e| e.to_string())?; if !output.status.success() { @@ -965,6 +1031,7 @@ pub async fn cli_execute_query( cmd.stdout(Stdio::piped()); cmd.stderr(Stdio::piped()); + no_console_window(&mut cmd); let start = std::time::Instant::now(); let mut child = cmd.spawn().map_err(|e| format!("Spawn failed: {}", e))?; @@ -1060,6 +1127,7 @@ pub async fn cli_list_databases( _ => unreachable!(), } + no_console_window(&mut cmd); let output = cmd.output().await.map_err(|e| e.to_string())?; if !output.status.success() { diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index b8654d6..37a7537 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -77,6 +77,7 @@ pub fn run() { get_build_info, cli::cli_check_tools, cli::cli_list_cached, + cli::cli_remove_cached, cli::cli_check_tool, cli::cli_check_system_tool, cli::cli_ensure, diff --git a/src-tauri/src/storage.rs b/src-tauri/src/storage.rs index 52d4b3b..5b9070c 100644 --- a/src-tauri/src/storage.rs +++ b/src-tauri/src/storage.rs @@ -134,6 +134,20 @@ fn get_machine_id() -> String { return cached.clone(); } + // Windows machine-id detection shells out to PowerShell/WMI, which can be + // slow or transiently unavailable (AV/EDR hooks, WMI service not yet up + // on a fresh boot). A different result across app launches silently + // rotates the encryption key, so once we detect a real ID we persist it + // in the OS keyring and prefer that value over live re-detection. + if let Ok(entry) = Entry::new("queryden", "machine_id_cache") { + if let Ok(id) = entry.get_password() { + if !id.is_empty() { + let _ = MACHINE_ID_CACHE.set(id.clone()); + return id; + } + } + } + let id = detect_machine_id(); // Only memoize real IDs. A transient detection failure (e.g. PowerShell @@ -141,6 +155,11 @@ fn get_machine_id() -> String { // fallback sentinel — that would silently weaken every subsequent key. if id != FALLBACK_MACHINE_ID { let _ = MACHINE_ID_CACHE.set(id.clone()); + if let Ok(entry) = Entry::new("queryden", "machine_id_cache") { + if let Err(e) = entry.set_password(&id) { + warn!("Failed to persist machine ID to OS keyring: {e}"); + } + } } id @@ -1276,11 +1295,15 @@ pub fn load_saved_queries(app: tauri::AppHandle) -> Result, match serde_json::from_str::(&json) { Ok(data) => { if data.machine_fingerprint != get_machine_fingerprint() { + warn!("saved-queries.json machine fingerprint mismatch — refusing to load (file was created on a different machine, or machine ID detection is unstable on this one)"); return Ok(vec![]); } Ok(data.queries) }, - Err(_) => Ok(vec![]) + Err(e) => { + warn!("saved-queries.json failed to decrypt/parse — treating as empty: {e}"); + Ok(vec![]) + } } } diff --git a/src/components/editor/QueryEditor.tsx b/src/components/editor/QueryEditor.tsx index f4f965d..d700249 100644 --- a/src/components/editor/QueryEditor.tsx +++ b/src/components/editor/QueryEditor.tsx @@ -14,6 +14,7 @@ import { matchesQualifiedOrBareName, } from "./completionContext"; import { resolveStatementAtOffset } from "../../utils/statementAtCursor"; +import { splitStatements } from "../../utils/splitStatements"; // Global tracking to prevent duplicate provider registration across component mounts let sqlProviderDisposable: any = null; @@ -611,36 +612,18 @@ export const QueryEditor = memo(function QueryEditor({ const model = editor.getModel(); if (!model) return; - const text = model.getValue().trim(); + const rawText = model.getValue(); + const text = rawText.trim(); if (!text) { onRunRef.current?.(); return; } // Collect all statements with their line numbers - const allStatements: { text: string; lineNumber: number }[] = []; - let searchFrom = 0; - - while (searchFrom < text.length) { - const semiPos = text.indexOf(';', searchFrom); - let endPos = semiPos === -1 ? text.length : semiPos; - - // Skip leading whitespace for this statement - let startPos = searchFrom; - while (startPos < endPos && /\s/.test(text[startPos])) { - startPos++; - } - - // Extract statement - const statement = text.substring(startPos, endPos).trim(); - if (statement) { - const statementStartPos = model.getPositionAt(startPos); - allStatements.push({ text: statement, lineNumber: statementStartPos.lineNumber }); - } - - if (semiPos === -1) break; - searchFrom = semiPos + 1; - } + // Use the untrimmed text so splitStatements' line number computation + // matches Monaco's 1-based line numbering (trim loses leading blanks). + const parsed = splitStatements(rawText); + const allStatements = parsed.map(s => ({ text: s.text, lineNumber: s.lineNumber })); // Run all statements - pass special flag to executeQuery if (allStatements.length > 0) { diff --git a/src/components/explorer/DatabaseExplorer.tsx b/src/components/explorer/DatabaseExplorer.tsx index 5024986..4741b3f 100644 --- a/src/components/explorer/DatabaseExplorer.tsx +++ b/src/components/explorer/DatabaseExplorer.tsx @@ -1646,15 +1646,22 @@ export function DatabaseExplorer({ isAddConnectionDialogOpen = false }: Database try { let iconType = node.icon; let targetName = node.name; - const idParts = node.id.split("-"); - if (idParts.length >= 2) { - const fullPath = idParts.slice(1).join("-"); - if (fullPath.includes(".")) { - targetName = fullPath; - if (node.icon === "column") { - const pathParts = fullPath.split("-"); - targetName = pathParts[0]; - iconType = "table"; + // Triggers are looked up by plain tgname (pg_trigger has no + // schema-qualified name), so skip the schema.name path + // reconstruction below — for a table-nested trigger, node.id + // is `trig-schema.table-triggerName`, and reconstructing the + // path would wrongly fold the table name into the trigger name. + if (node.icon !== "trigger") { + const idParts = node.id.split("-"); + if (idParts.length >= 2) { + const fullPath = idParts.slice(1).join("-"); + if (fullPath.includes(".")) { + targetName = fullPath; + if (node.icon === "column") { + const pathParts = fullPath.split("-"); + targetName = pathParts[0]; + iconType = "table"; + } } } } diff --git a/src/components/layout/MainContent.tsx b/src/components/layout/MainContent.tsx index e7d82dd..05b826f 100644 --- a/src/components/layout/MainContent.tsx +++ b/src/components/layout/MainContent.tsx @@ -517,7 +517,6 @@ export function MainContent() { useEffect(() => { if (!initialLoadDone) return; if (sessionRestoredRef.current) return; - sessionRestoredRef.current = true; (async () => { try { const { invokeCmd } = await import("../../lib/ipc"); @@ -542,6 +541,8 @@ export function MainContent() { logger.debug(`Restored ${restored.length} tabs from session`); } catch (e) { logger.debug("No saved session to restore:", e); + } finally { + sessionRestoredRef.current = true; } })(); }, [initialLoadDone]); // eslint-disable-line react-hooks/exhaustive-deps @@ -549,6 +550,11 @@ export function MainContent() { // ── Session persistence: save tabs whenever they change ────────────────── const sessionSaveTimerRef = useRef | null>(null); useEffect(() => { + // Don't save until the restore attempt above has completed — otherwise + // this fires on mount with the initial empty `queryTabs` and can win + // the race against the (async) restore, wiping sessions.json with an + // empty tab list before it's ever read back. + if (!sessionRestoredRef.current) return; if (sessionSaveTimerRef.current) clearTimeout(sessionSaveTimerRef.current); sessionSaveTimerRef.current = setTimeout(async () => { try { @@ -995,13 +1001,15 @@ const executeQuery = useCallback(async (specificQuery?: any, statementInfo?: { l // Compute query type immediately (needed by both libpq and CLI paths) const upperQuery = queryToRun.toUpperCase(); const cleanUpper = queryToRun.replace(/--.*$/gm, "").replace(/\/\*[\s\S]*?\*\//g, "").trim().toUpperCase(); + const isDoBlock = /^DO\s+[$']/.test(cleanUpper); const isSelect = - cleanUpper.startsWith("SELECT") || + (cleanUpper.startsWith("SELECT") || cleanUpper.includes("RETURNING") || cleanUpper.startsWith("WITH") || cleanUpper.startsWith("SHOW") || cleanUpper.startsWith("EXPLAIN") || - cleanUpper.includes("(SELECT"); + cleanUpper.includes("(SELECT")) && + !isDoBlock; const isTruncate = upperQuery.includes("TRUNCATE"); const isDelete = upperQuery.includes("DELETE"); const hasWhere = upperQuery.includes("WHERE"); @@ -1353,13 +1361,16 @@ const executeQuery = useCallback(async (specificQuery?: any, statementInfo?: { l const stmtInfo = statementInfos[i]; const lineNumber = stmtInfo?.lineNumber || 1; const stmtUpper = stmt.toUpperCase().trim(); + const cleanStmt = stmt.replace(/--.*$/gm, "").replace(/\/\*[\s\S]*?\*\//g, "").trim().toUpperCase(); + const isDoBlock = /^DO\s+[$']/.test(cleanStmt); const isStmtSelect = - stmtUpper.startsWith("SELECT") || + (stmtUpper.startsWith("SELECT") || stmtUpper.includes("RETURNING") || stmtUpper.startsWith("WITH") || stmtUpper.startsWith("SHOW") || stmtUpper.startsWith("EXPLAIN") || - stmtUpper.includes("(SELECT"); + stmtUpper.includes("(SELECT")) && + !isDoBlock; try { const stmtStartTime = Date.now(); if (isStmtSelect) { @@ -1515,7 +1526,7 @@ const executeQuery = useCallback(async (specificQuery?: any, statementInfo?: { l } else { const { rowsAffected: affected } = await cliExecStmt(queryToRun, false, currentPsqlExpanded, currentCliDatabase || ""); rowsAffected = affected ?? 0; - setSuccess(`Query executed successfully. ${rowsAffected} rows affected.`); + setSuccess(isDoBlock ? "DO" : `Query executed successfully. ${rowsAffected} rows affected.`); rows = []; statementResults.push({ lineNumber: stmtInfo.lineNumber, status: 'success', rowsAffected, executionTime: Date.now() - stmtStartTime }); if (currentTabId) updateTabState(currentTabId, { statementResults }); @@ -1671,13 +1682,16 @@ const executeQuery = useCallback(async (specificQuery?: any, statementInfo?: { l const lineNumber = stmtInfo?.lineNumber || 1; const stmtUpper = stmt.toUpperCase().trim(); + const cleanStmt = stmt.replace(/--.*$/gm, "").replace(/\/\*[\s\S]*?\*\//g, "").trim().toUpperCase(); + const isDoBlock = /^DO\s+[$']/.test(cleanStmt); const isStmtSelect = - stmtUpper.startsWith("SELECT") || + (stmtUpper.startsWith("SELECT") || stmtUpper.includes("RETURNING") || stmtUpper.startsWith("WITH") || // CTE queries stmtUpper.startsWith("SHOW") || stmtUpper.startsWith("EXPLAIN") || - stmtUpper.includes("(SELECT"); // Subqueries + stmtUpper.includes("(SELECT")) && // Subqueries + !isDoBlock; try { const stmtStartTime = Date.now(); @@ -1763,7 +1777,7 @@ const executeQuery = useCallback(async (specificQuery?: any, statementInfo?: { l } else { const result = await db.execute(queryToRun); rowsAffected = typeof result.rowsAffected === 'number' ? result.rowsAffected : 0; - setSuccess(`Query executed successfully. ${rowsAffected} rows affected.`); + setSuccess(isDoBlock ? "DO" : `Query executed successfully. ${rowsAffected} rows affected.`); rows = []; statementResults.push({ lineNumber: stmtInfo.lineNumber, @@ -2065,12 +2079,25 @@ const executeQuery = useCallback(async (specificQuery?: any, statementInfo?: { l cancelFlagRef.current = false; setIsExecuting(false); isExecutingRef.current = false; + } else { + // Stale (cancelled) run: free the execution gate so new runs can + // start, but leave cancelFlagRef alone — a newer run already + // cleared it (or will set its own). + isExecutingRef.current = false; } } }, [activeConnection, selectedDatabase, addQuery, currentDb, vaultCredentials, settings, confirmDialog]); const cancelQuery = useCallback(() => { cancelFlagRef.current = true; + // Don't clear isExecutingRef here — doing so would let a new run start + // before the cancelled run's async loop settles. The new run resets + // cancelFlagRef (line 966), which would let the stale run continue + // executing SQL after the cancel (side effects!). + // + // Bump generation so the abandoned run's finally block no-ops + // instead of clobbering state from a newer run. + executionGenRef.current++; setIsExecuting(false); setError("Query execution cancelled by user."); setExecutionTime(runningTimeMs); diff --git a/src/components/settings/SettingsDialog.tsx b/src/components/settings/SettingsDialog.tsx index bd8fbf6..15d1480 100644 --- a/src/components/settings/SettingsDialog.tsx +++ b/src/components/settings/SettingsDialog.tsx @@ -1,18 +1,20 @@ import { useState, useEffect } from "react"; -import { X, Settings, Database, Play, Monitor, Keyboard, Zap, Trash2, Search, Import, Edit2, Copy, HardDrive, Shield, AlertTriangle, CheckCircle, AlertCircle, Download } from "lucide-react"; +import { X, Settings, Database, Play, Monitor, Keyboard, Zap, Trash2, Search, Import, Edit2, Copy, HardDrive, Shield, AlertTriangle, CheckCircle, AlertCircle, Download, Terminal, RefreshCw } from "lucide-react"; import { useSettings } from "../../store/settingsStore"; import { useKeymap, defaultKeymaps, useLiveTemplates } from "../../store/keymapStore"; import { useAI, AIProvider } from "../../store/aiStore"; +import { useCliStore } from "../../store/cliStore"; import { Sparkles, Bot } from "lucide-react"; import { VaultCredential } from "../../contexts/ConnectionContext"; import { useConnections } from "../../contexts/useConnections"; import { useVault } from "../../store/vaultStore"; +import { useConfirmDialog } from "../ui/ConfirmDialog"; import { PasswordInput } from "../ui/PasswordInput"; import { Button } from "../ui/Button"; import { IconButton } from "../ui/IconButton"; import { Select } from "../ui/Select"; -type SettingsCategory = "appearance" | "sqlCompletion" | "queryExecution" | "explorer" | "keymap" | "templates" | "importExport" | "ai" | "copyTransfer" | "permissions" | "vault" | "updates" | "autoSave"; +type SettingsCategory = "appearance" | "sqlCompletion" | "queryExecution" | "explorer" | "keymap" | "templates" | "importExport" | "ai" | "copyTransfer" | "permissions" | "vault" | "updates" | "autoSave" | "cliTools"; interface SettingsDialogProps { isOpen: boolean; @@ -32,6 +34,7 @@ const categories: { id: SettingsCategory; label: string; icon: any; keywords: st { id: "vault", label: "Credential Vault", icon: Shield, keywords: ["vault", "security", "credentials", "password", "username", "encryption", "master", "profiles"] }, { id: "updates", label: "Updates", icon: Download, keywords: ["update", "updates", "beta", "channel", "stable", "release", "version", "auto-update", "prerelease"] }, { id: "autoSave", label: "Auto Save", icon: HardDrive, keywords: ["auto-save", "autosave", "save", "interval", "recovery", "backup", "persist", "crash"] }, + { id: "cliTools", label: "CLI Tools", icon: Terminal, keywords: ["cli", "psql", "mysql", "mongo", "mongosh", "redis", "download", "tools", "binary", "cache"] }, ]; export function SettingsDialog({ isOpen, onClose }: SettingsDialogProps) { @@ -139,6 +142,7 @@ export function SettingsDialog({ isOpen, onClose }: SettingsDialogProps) { {activeCategory === "vault" && } {activeCategory === "updates" && } {activeCategory === "autoSave" && } + {activeCategory === "cliTools" && } {/* Footer */} @@ -1461,3 +1465,189 @@ function ChannelOption({ value, current, label, description, onChange }: { ); } + +const CLI_TOOL_KINDS: { kind: string; label: string; binary: string; defaultVersion: number }[] = [ + { kind: "postgresql", label: "PostgreSQL", binary: "psql", defaultVersion: 16 }, + { kind: "mysql", label: "MySQL", binary: "mysql", defaultVersion: 9 }, + { kind: "mongodb", label: "MongoDB", binary: "mongosh", defaultVersion: 2 }, + { kind: "redis", label: "Redis", binary: "redis-cli", defaultVersion: 7 }, +]; + +function formatBytes(bytes: number): string { + if (bytes <= 0) return "0 MB"; + const mb = bytes / (1024 * 1024); + if (mb < 1024) return `${mb.toFixed(1)} MB`; + return `${(mb / 1024).toFixed(2)} GB`; +} + +function CliToolsSettings() { + const cli = useCliStore(); + const confirmDialog = useConfirmDialog(); + const [refreshing, setRefreshing] = useState(false); + const [downloadingKind, setDownloadingKind] = useState(null); + const [downloadError, setDownloadError] = useState(null); + const [removingKey, setRemovingKey] = useState(null); + const [versionInputs, setVersionInputs] = useState>({}); + + const refresh = async () => { + setRefreshing(true); + try { + await Promise.all([cli.fetchTools(), cli.listCached()]); + } finally { + setRefreshing(false); + } + }; + + useEffect(() => { + refresh(); + // eslint-disable-next-line react-hooks/exhaustive-deps + }, []); + + const handleDownload = async (kind: string, defaultVersion: number) => { + const raw = versionInputs[kind]; + const majorVersion = raw ? parseInt(raw, 10) : defaultVersion; + if (Number.isNaN(majorVersion) || majorVersion <= 0) return; + setDownloadError(null); + setDownloadingKind(kind); + try { + await cli.downloadVersion(kind, majorVersion); + await cli.listCached(); + } catch (e) { + setDownloadError(String(e)); + } finally { + setDownloadingKind(null); + } + }; + + const handleRemove = async (kind: string, majorVersion: number) => { + const key = `${kind}-${majorVersion}`; + const confirmed = await confirmDialog.confirm({ + title: "Remove cached CLI tool", + message: `Remove the cached ${kind} ${majorVersion} binaries from disk? You can re-download it later if needed.`, + confirmLabel: "Remove", + type: "danger", + }); + if (!confirmed) return; + setRemovingKey(key); + try { + await cli.removeCached(kind, majorVersion); + } catch (e) { + console.error("Failed to remove cached CLI tool:", e); + } finally { + setRemovingKey(null); + } + }; + + return ( +
+
+

CLI Tools

+ +
+

+ Manage the database CLIs QueryDen uses for the psql console and CLI-based query execution. + Pre-download a version here to avoid waiting mid-query, or free up disk space by removing + cached versions you no longer use. Downloading during query execution remains available as + a fallback if a tool isn't set up here. +

+ + {downloadError && ( +
+ + {downloadError} +
+ )} + +
+ {CLI_TOOL_KINDS.map(({ kind, label, binary, defaultVersion }) => { + const systemInfo = cli.tools.find((t) => t.kind === kind); + const cachedVersions = cli.cachedTools.filter((t) => t.kind === kind); + const isDownloading = downloadingKind === kind; + + return ( +
+
+
+
{label}
+
{binary}
+
+ {systemInfo?.available ? ( +
+ + On system PATH +
+ ) : ( +
+ + Not on system PATH +
+ )} +
+ + {systemInfo?.path && ( +
+ {systemInfo.path} +
+ )} + + {cachedVersions.length > 0 && ( +
+ {cachedVersions.map((cached) => { + const key = `${cached.kind}-${cached.majorVersion}`; + return ( +
+
+ v{cached.majorVersion} + {formatBytes(cached.sizeBytes)} +
+ } + label="Remove cached version" + size="sm" + variant="ghost" + disabled={removingKey === key} + onClick={() => handleRemove(cached.kind, cached.majorVersion)} + /> +
+ ); + })} +
+ )} + +
+ setVersionInputs((prev) => ({ ...prev, [kind]: e.target.value }))} + className="w-20 px-2 py-1 text-xs rounded bg-[var(--surface-base)] border border-[var(--neutral-6)] outline-none focus:border-[var(--accent-8)] text-[var(--neutral-12)]" + /> + +
+
+ ); + })} +
+
+ ); +} diff --git a/src/lib/ipc.ts b/src/lib/ipc.ts index 8482ef5..bed831e 100644 --- a/src/lib/ipc.ts +++ b/src/lib/ipc.ts @@ -137,6 +137,7 @@ export interface CachedToolDto { major_version: number; binaries: string[]; path: string; + size_bytes: number; } /** Shape returned by `cli_check_tool` (built via `serde_json::json!`, camelCase). */ @@ -318,6 +319,10 @@ export interface IpcCommands { // cli cli_check_tools: { args: void; result: CliToolInfoDto[] }; cli_list_cached: { args: void; result: CachedToolDto[] }; + cli_remove_cached: { + args: { toolKind: string; majorVersion: number }; + result: void; + }; cli_check_tool: { args: { toolKind: string; majorVersion: number }; result: CheckToolResult; diff --git a/src/store/cliStore.ts b/src/store/cliStore.ts index 03fcbcd..ca61508 100644 --- a/src/store/cliStore.ts +++ b/src/store/cliStore.ts @@ -21,6 +21,7 @@ export interface CachedTool { majorVersion: number; binaries: string[]; path: string; + sizeBytes: number; } // Re-export for back-compat with components that import the type from this module. @@ -33,6 +34,7 @@ interface CliStore { error: string | null; fetchTools: () => Promise; listCached: () => Promise; + removeCached: (kind: string, majorVersion: number) => Promise; checkTool: (kind: string, majorVersion: number) => Promise; checkSystemTool: (kind: string) => Promise; ensureTool: (kind: string, majorVersion?: number) => Promise; @@ -92,6 +94,7 @@ function toCachedTool(t: CachedToolDto): CachedTool { majorVersion: t.major_version, binaries: t.binaries, path: t.path, + sizeBytes: t.size_bytes, }; } @@ -118,6 +121,12 @@ export const useCliStore = create((set, get) => ({ return tools; }, + removeCached: async (kind, majorVersion) => { + await invokeCmd("cli_remove_cached", { toolKind: kind, majorVersion }); + await get().listCached(); + await get().fetchTools(); + }, + checkTool: (kind, majorVersion) => invokeCmd("cli_check_tool", { toolKind: kind, majorVersion }), diff --git a/src/store/savedQueryStore.ts b/src/store/savedQueryStore.ts index bbcfe97..4a5b194 100644 --- a/src/store/savedQueryStore.ts +++ b/src/store/savedQueryStore.ts @@ -41,7 +41,8 @@ const loadFromFile = async (): Promise => { connectionId: q.connection_id, createdAt: q.created_at, })); - } catch { + } catch (e) { + logger.error("Failed to load saved queries:", e); return []; } }; diff --git a/src/utils/applyQueryLimit.test.ts b/src/utils/applyQueryLimit.test.ts index 1fb1cef..3e823da 100644 --- a/src/utils/applyQueryLimit.test.ts +++ b/src/utils/applyQueryLimit.test.ts @@ -83,5 +83,35 @@ describe("applyQueryLimit", () => { const union = "SELECT 1 UNION SELECT 2"; expect(applyQueryLimit(union, 1000)).toBe(union); }); + + it("leaves DO blocks with RETURNING unchanged", () => { + const doBlock = `DO $$ + DECLARE + old_type_id INTEGER; + BEGIN + SELECT id INTO old_type_id FROM t; + UPDATE t SET name = 'foo' WHERE id = old_type_id RETURNING id INTO old_type_id; + END; +$$;`; + expect(applyQueryLimit(doBlock, 1000)).toBe(doBlock); + }); + + it("leaves DO blocks with named dollar-quoting unchanged", () => { + const doBlock = `DO $body$ + BEGIN + INSERT INTO t (name) VALUES ('x') RETURNING id; + END; +$body$;`; + expect(applyQueryLimit(doBlock, 1000)).toBe(doBlock); + }); + + it("leaves DO LANGUAGE plpgsql blocks unchanged", () => { + const doBlock = `DO LANGUAGE plpgsql $$ + BEGIN + INSERT INTO t (name) VALUES ('x') RETURNING id; + END; +$$;`; + expect(applyQueryLimit(doBlock, 1000)).toBe(doBlock); + }); }); }); diff --git a/src/utils/applyQueryLimit.ts b/src/utils/applyQueryLimit.ts index 38885b7..f329faa 100644 --- a/src/utils/applyQueryLimit.ts +++ b/src/utils/applyQueryLimit.ts @@ -18,6 +18,11 @@ export function applyQueryLimit(query: string, maxRows: number): string { .trim() .toUpperCase(); + // Skip PL/pgSQL anonymous blocks (DO $$ ... $$; or DO LANGUAGE plpgsql $$ ... $$;) + if (/^DO(\s+LANGUAGE\s+\w+)?\s+[$']/.test(cleanQuery)) { + return query; + } + if ( !cleanQuery.startsWith("SELECT") && !cleanQuery.includes("RETURNING") && From c80ed17dc7efd7a0e74b6b10e63ac95fd2f7c1f4 Mon Sep 17 00:00:00 2001 From: "coderabbitai[bot]" <136622811+coderabbitai[bot]@users.noreply.github.com> Date: Wed, 29 Jul 2026 08:40:09 +0000 Subject: [PATCH 2/4] fix: apply CodeRabbit auto-fixes Fixed 2 file(s) based on 1 unresolved review comment. Co-authored-by: CodeRabbit --- src/components/explorer/DatabaseExplorer.tsx | 35 ++++++++++++++++---- src/contexts/ConnectionContext.tsx | 24 +++++++++++--- 2 files changed, 47 insertions(+), 12 deletions(-) diff --git a/src/components/explorer/DatabaseExplorer.tsx b/src/components/explorer/DatabaseExplorer.tsx index 4741b3f..a100d19 100644 --- a/src/components/explorer/DatabaseExplorer.tsx +++ b/src/components/explorer/DatabaseExplorer.tsx @@ -1646,12 +1646,33 @@ export function DatabaseExplorer({ isAddConnectionDialogOpen = false }: Database try { let iconType = node.icon; let targetName = node.name; - // Triggers are looked up by plain tgname (pg_trigger has no - // schema-qualified name), so skip the schema.name path - // reconstruction below — for a table-nested trigger, node.id - // is `trig-schema.table-triggerName`, and reconstructing the - // path would wrongly fold the table name into the trigger name. - if (node.icon !== "trigger") { + let triggerSchema: string | undefined; + let triggerTable: string | undefined; + + // For triggers, extract schema and table from node.id to disambiguate + // lookups when the same trigger name exists on different tables. + // Table-nested trigger: node.id = `trig-schema.table-triggerName` + // Schema-level trigger: node.id = `trig-schema.triggerName` + if (node.icon === "trigger") { + const idParts = node.id.split("-"); + if (idParts.length === 3) { + // Table-nested: ["trig", "schema.table", "triggerName"] + const pathPart = idParts[1]; + if (pathPart.includes(".")) { + const dotParts = pathPart.split("."); + triggerSchema = dotParts[0]; + triggerTable = dotParts.slice(1).join("."); + } + } else if (idParts.length === 2) { + // Schema-level: ["trig", "schema.triggerName"] + const pathPart = idParts[1]; + if (pathPart.includes(".")) { + const dotParts = pathPart.split("."); + triggerSchema = dotParts[0]; + // No table for schema-level triggers + } + } + } else { const idParts = node.id.split("-"); if (idParts.length >= 2) { const fullPath = idParts.slice(1).join("-"); @@ -1665,7 +1686,7 @@ export function DatabaseExplorer({ isAddConnectionDialogOpen = false }: Database } } } - const ddl = await getDDL(iconType, targetName); + const ddl = await getDDL(iconType, targetName, triggerSchema, triggerTable); if (ddl) { window.dispatchEvent(new CustomEvent("open-query-with-text", { detail: { query: ddl, name: `DDL ${targetName}` } diff --git a/src/contexts/ConnectionContext.tsx b/src/contexts/ConnectionContext.tsx index ce7ced6..ccd5cbc 100644 --- a/src/contexts/ConnectionContext.tsx +++ b/src/contexts/ConnectionContext.tsx @@ -139,7 +139,7 @@ interface ConnectionContextType { connectToDatabase: (connId: string, databaseName?: string, overrideVaultCredential?: VaultCredential) => Promise; disconnectFromDatabase: () => Promise; loadSchema: (database: string, overrideSchemas?: string[]) => Promise; - getDDL: (type: string, name: string) => Promise; + getDDL: (type: string, name: string, schema?: string, table?: string) => Promise; generateStatement: (type: "select" | "insert" | "update" | "delete", tableName: string) => Promise; exportConnections: (path: string, includePasswords: boolean) => Promise; importConnections: (path: string) => Promise; @@ -1096,11 +1096,11 @@ export function ConnectionProvider({ children }: { children: ReactNode }) { return ddl; }; - const getDDL = async (type: string, name: string): Promise => { + const getDDL = async (type: string, name: string, schema?: string, table?: string): Promise => { if (!activeConnection || !currentDb) return ""; - + // Parse name - could be "table" or "schema.table" - let schemaPart = 'public'; + let schemaPart = schema || 'public'; let tablePart = name; // Handle case like "ansible.job" or "public.job" @@ -1330,7 +1330,21 @@ export function ConnectionProvider({ children }: { children: ReactNode }) { } } else if (type === "trigger") { try { - const result = await currentDb.select("SELECT pg_get_triggerdef(oid) FROM pg_trigger WHERE tgname = $1", [name]); + let result; + // If schema and table are provided, constrain lookup to avoid ambiguity + // when the same trigger name exists on different tables + if (schema && table) { + result = await currentDb.select(` + SELECT pg_get_triggerdef(t.oid) + FROM pg_trigger t + JOIN pg_class c ON t.tgrelid = c.oid + JOIN pg_namespace n ON c.relnamespace = n.oid + WHERE t.tgname = $1 AND n.nspname = $2 AND c.relname = $3 + `, [name, schema, table]); + } else { + // Fallback: look up by trigger name alone (may be ambiguous) + result = await currentDb.select("SELECT pg_get_triggerdef(oid) FROM pg_trigger WHERE tgname = $1", [name]); + } if (result && result.length > 0) { const def = Object.values(result[0])[0]; return typeof def === 'string' ? def : `-- Trigger definition found but is not a string`; From fad365e35553a01c087a070c6dc148072eb7ccd0 Mon Sep 17 00:00:00 2001 From: "coderabbitai[bot]" <136622811+coderabbitai[bot]@users.noreply.github.com> Date: Wed, 29 Jul 2026 09:25:01 +0000 Subject: [PATCH 3/4] fix: apply CodeRabbit auto-fixes Fixed 3 file(s) based on 4 unresolved review comments. Co-authored-by: CodeRabbit --- src-tauri/src/storage.rs | 20 +++++++++++ src/components/layout/MainContent.tsx | 42 +++++++++++++++++----- src/components/settings/SettingsDialog.tsx | 7 +++- 3 files changed, 59 insertions(+), 10 deletions(-) diff --git a/src-tauri/src/storage.rs b/src-tauri/src/storage.rs index 5b9070c..910eb90 100644 --- a/src-tauri/src/storage.rs +++ b/src-tauri/src/storage.rs @@ -84,6 +84,11 @@ const FALLBACK_MACHINE_ID: &str = "default-machine-id"; // result so the cost is paid once per process. static MACHINE_ID_CACHE: OnceLock = OnceLock::new(); +// Lock guarding machine ID initialization to prevent concurrent cache-miss +// races (issue: concurrent threads could all miss the cache, run detection, +// and then race to persist different values to the keyring). +static MACHINE_ID_INIT_LOCK: Mutex<()> = Mutex::new(()); + // Process-wide cache for the *derived* 32-byte encryption key. // // `get_encryption_key()` runs Argon2id on every call. With OWASP defaults @@ -130,6 +135,18 @@ fn vault_password_fingerprint(pwd: &str) -> [u8; 32] { // Use `try_get_machine_id()` for new encryption — it errors instead of weakening // the key with a globally-known sentinel. fn get_machine_id() -> String { + // Fast path: if cache is already populated, return immediately. + if let Some(cached) = MACHINE_ID_CACHE.get() { + return cached.clone(); + } + + // Acquire the init lock to serialize cache-miss initialization (double- + // checked locking pattern). Only one thread will perform detection and + // persist to the keyring; others will wait and then recheck the cache. + let _guard = MACHINE_ID_INIT_LOCK.lock().unwrap(); + + // Recheck the cache after acquiring the lock — a concurrent thread may + // have already populated it while we were waiting. if let Some(cached) = MACHINE_ID_CACHE.get() { return cached.clone(); } @@ -154,6 +171,9 @@ fn get_machine_id() -> String { // missing from PATH momentarily) must not lock the process into the // fallback sentinel — that would silently weaken every subsequent key. if id != FALLBACK_MACHINE_ID { + // The lock is still held, so only the winning thread persists to both + // the cache and the keyring. Concurrent losers get nothing — they'll + // read the winner's cached value on their cache recheck above. let _ = MACHINE_ID_CACHE.set(id.clone()); if let Ok(entry) = Entry::new("queryden", "machine_id_cache") { if let Err(e) = entry.set_password(&id) { diff --git a/src/components/layout/MainContent.tsx b/src/components/layout/MainContent.tsx index 05b826f..ef12f60 100644 --- a/src/components/layout/MainContent.tsx +++ b/src/components/layout/MainContent.tsx @@ -164,6 +164,10 @@ export function MainContent() { const isExecutingRef = useRef(false); const runningCmdRef = useRef(""); const executionGenRef = useRef(0); + // Per-run cancel token: holds a mutable flag for the currently-executing + // run. When a new run starts it creates a fresh token; when the user + // cancels, we set the current token to true (but leave old tokens alone). + const currentRunCancelRef = useRef<{ current: boolean } | null>(null); // Ref for latest activeTab to avoid stale closures in executeQuery const activeTabRef = useRef(undefined); const activeTabIdRef = useRef(undefined); @@ -964,8 +968,14 @@ const executeQuery = useCallback(async (specificQuery?: any, statementInfo?: { l setSuccess(null); setMultiResults([]); setRunningTimeMs(0); - cancelFlagRef.current = false; - + + // Per-run cancellation: each run gets its own cancel token. When a new + // run starts it does NOT clear the global cancelFlagRef immediately — + // that would let a previously-cancelled run continue executing. Instead + // we create a fresh local cancel flag and only check that one. + const runCancelled = { current: false }; + currentRunCancelRef.current = runCancelled; + // Clear statement results when starting new execution (glyphs will appear after execution completes) if (currentTabId) { updateTabState(currentTabId, { statementResults: [] }); @@ -1675,7 +1685,9 @@ const executeQuery = useCallback(async (specificQuery?: any, statementInfo?: { l // Handle multiple statements (Ctrl+Shift+Enter) if (isRunAll && statementsToRun.length > 0) { for (let i = 0; i < statementsToRun.length; i++) { - if (cancelFlagRef.current) break; + // Check the per-run cancel token, not the global one. This + // prevents a cancelled run from continuing after a new run starts. + if (runCancelled.current) break; const stmt = statementsToRun[i]; const stmtInfo = statementInfos[i]; @@ -1805,7 +1817,9 @@ const executeQuery = useCallback(async (specificQuery?: any, statementInfo?: { l if (intervalId) clearInterval(intervalId); intervalId = null; } - if (cancelFlagRef.current || gen !== executionGenRef.current) return; + // Check the per-run cancel token (not the global one) to decide + // whether to bail early. A stale generation also bails. + if (runCancelled.current || gen !== executionGenRef.current) return; const duration = Date.now() - startTime; @@ -2089,14 +2103,24 @@ const executeQuery = useCallback(async (specificQuery?: any, statementInfo?: { l }, [activeConnection, selectedDatabase, addQuery, currentDb, vaultCredentials, settings, confirmDialog]); const cancelQuery = useCallback(() => { + // Set the current run's per-run cancel token. This prevents the cancelled + // run from continuing to execute statements in its run-all loop, while + // NOT affecting any future runs (they get their own fresh tokens). + if (currentRunCancelRef.current) { + currentRunCancelRef.current.current = true; + } + // Legacy global flag kept for backwards compatibility with any code + // that still checks it (though the per-run token is the source of truth). cancelFlagRef.current = true; // Don't clear isExecutingRef here — doing so would let a new run start - // before the cancelled run's async loop settles. The new run resets - // cancelFlagRef (line 966), which would let the stale run continue - // executing SQL after the cancel (side effects!). + // before the cancelled run's async loop settles. The execution gate + // stays held until the cancelled run's finally block runs (which checks + // the generation and only clears isExecutingRef for stale runs). // - // Bump generation so the abandoned run's finally block no-ops - // instead of clobbering state from a newer run. + // Bump generation so the abandoned run's finally block no-ops UI state + // updates (it won't clear cancelFlagRef or setIsExecuting, but WILL + // clear isExecutingRef so new runs can start once the stale async + // work settles). executionGenRef.current++; setIsExecuting(false); setError("Query execution cancelled by user."); diff --git a/src/components/settings/SettingsDialog.tsx b/src/components/settings/SettingsDialog.tsx index 15d1480..07a7152 100644 --- a/src/components/settings/SettingsDialog.tsx +++ b/src/components/settings/SettingsDialog.tsx @@ -1531,8 +1531,13 @@ function CliToolsSettings() { setRemovingKey(key); try { await cli.removeCached(kind, majorVersion); + await cli.listCached(); // Refresh the cached tools list after successful removal } catch (e) { - console.error("Failed to remove cached CLI tool:", e); + // Surface the error to the user using the same downloadError state + // mechanism used by handleDownload. The error banner is already + // wired to display at the top of the CLI Tools settings section. + const errMsg = e instanceof Error ? e.message : String(e); + setDownloadError(`Failed to remove ${kind} v${majorVersion}: ${errMsg}`); } finally { setRemovingKey(null); } From 9b055100f4d4257929326f8df1198a0cd08f6a99 Mon Sep 17 00:00:00 2001 From: Keenan Simpson Date: Wed, 29 Jul 2026 13:35:24 +0200 Subject: [PATCH 4/4] Clarify trigger node.id format contract and cancelFlagRef commentary MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Document the two trigger node.id formats (schema-level vs table-nested) and the limitation with in quoted identifiers - Clarify that cancelFlagRef is not legacy — it is still the primary cancel signal for the CLI psql path (run-all, \watch, post-exec bail, catch block) --- src/components/explorer/DatabaseExplorer.tsx | 19 ++++++++++++++----- src/components/layout/MainContent.tsx | 10 +++++----- 2 files changed, 19 insertions(+), 10 deletions(-) diff --git a/src/components/explorer/DatabaseExplorer.tsx b/src/components/explorer/DatabaseExplorer.tsx index a100d19..7df4a33 100644 --- a/src/components/explorer/DatabaseExplorer.tsx +++ b/src/components/explorer/DatabaseExplorer.tsx @@ -1649,10 +1649,18 @@ export function DatabaseExplorer({ isAddConnectionDialogOpen = false }: Database let triggerSchema: string | undefined; let triggerTable: string | undefined; - // For triggers, extract schema and table from node.id to disambiguate - // lookups when the same trigger name exists on different tables. - // Table-nested trigger: node.id = `trig-schema.table-triggerName` - // Schema-level trigger: node.id = `trig-schema.triggerName` + // Tree node IDs encode the schema context needed for unambiguous + // DDL lookup. Two formats: + // Table-nested: `trig-.-` + // (built at line 308 via `trig-${tableId}-${t}`, + // where tableId = ".
" contains no `-`) + // Schema-level: `trig-.` + // (built at line 355 via `trig-${schemaName}.${t}`) + // Splitting by `-` gives exactly 3 parts for table-nested + // and 2 for schema-level, unless a quoted identifier contains + // `-` (extremely rare in PostgreSQL). Unmatched formats fall + // back to a name-only lookup which is correct for schema-level + // triggers and avoids wrong results for table-nested ones. if (node.icon === "trigger") { const idParts = node.id.split("-"); if (idParts.length === 3) { @@ -1669,9 +1677,10 @@ export function DatabaseExplorer({ isAddConnectionDialogOpen = false }: Database if (pathPart.includes(".")) { const dotParts = pathPart.split("."); triggerSchema = dotParts[0]; - // No table for schema-level triggers } } + // Fallthrough: unexpected part count → use node.name only + // (triggerSchema/triggerTable stay undefined → name-only lookup) } else { const idParts = node.id.split("-"); if (idParts.length >= 2) { diff --git a/src/components/layout/MainContent.tsx b/src/components/layout/MainContent.tsx index ef12f60..9d388ab 100644 --- a/src/components/layout/MainContent.tsx +++ b/src/components/layout/MainContent.tsx @@ -2103,14 +2103,14 @@ const executeQuery = useCallback(async (specificQuery?: any, statementInfo?: { l }, [activeConnection, selectedDatabase, addQuery, currentDb, vaultCredentials, settings, confirmDialog]); const cancelQuery = useCallback(() => { - // Set the current run's per-run cancel token. This prevents the cancelled - // run from continuing to execute statements in its run-all loop, while - // NOT affecting any future runs (they get their own fresh tokens). + // Signal per-run cancel token so the libpq run-all loop stops early. + // Each execution creates its own token, so this only affects the current + // run — a later run gets a fresh uncancelled token. if (currentRunCancelRef.current) { currentRunCancelRef.current.current = true; } - // Legacy global flag kept for backwards compatibility with any code - // that still checks it (though the per-run token is the source of truth). + // Also set the global flag, which the CLI psql path (run-all, \watch, + // post-exec bail, catch block) still checks as its cancel signal. cancelFlagRef.current = true; // Don't clear isExecutingRef here — doing so would let a new run start // before the cancelled run's async loop settles. The execution gate