fix: deep-audit findings — multi-attr alter, RefCell, close-after-save

Six audit findings from 5-agent post-batch review:

- **PG bit(1) → Bool**: classify_type only matched bare "bit"; PG's
  format_type() returns "bit(1)" for length-1 BIT. Fixed cell editor
  rendering as text instead of checkbox.
- **PG ALTER COLUMN multi-attribute loss**: build_alter_column used
  cascading early returns — type → nullable → default → first wins.
  A single AlterColumn op carrying multiple changed attributes lost
  all but the first. Now returns Vec<String>; structure_tracker's
  materialize flattens. +1 unit test asserting all 3 statements emit.
- **PG missing ROLLBACK on COMMIT failure**: COMMIT failure left the
  pooled connection in an open transaction. Issue ROLLBACK before
  surfacing the failure.
- **on_structure_save_failed leak**: did not clear close_after_save
  / close_window_after_save on failure. A future unrelated
  SaveCompleted on another tab would spuriously close the window.
  Mirrors the SaveFailedForTab handler.
- **close_tabs_for_table leak**: Drop Table closed tabs through
  finish_close_workspace_tab without dropping their close_after_save
  entry. Same window-close-spuriously root cause.
- **DuplicateRow filter-model index bug**: read source row from
  current_result.rows by row_position, but row_position is the
  FilterListModel index. After sort/search, wrong row's values
  cloned. Now reads through row_object_at + cells_clone.
- **suppress_emit RefCell race**: clear_box can fire signals
  re-entrantly while a borrow_mut is live, panicking. Switched to
  Cell<bool> (no borrow guards, get/set only).
- **Query timeout doesn't cancel underlying future**: timeout branch
  now also fires token.cancel() so any cancellation logic in the
  pool / connection-monitor sees the same signal as a manual Cancel.
  sqlx still has no future-drop hook for in-flight queries; long-
  running queries on the server may continue until network notices.
- **Timeout icon**: appointment-soon-symbolic was a calendar icon;
  switched to dialog-warning-symbolic.

Cleared (not bugs): F5/disconnect race, multi-result signal leak,
single-instance lock timing, banner mutex flash, backslash injection,
update_added_columns_ops predicate (retain_ops semantics correct).

Author: datlechin

linux/crates/app/src/services/structure_tracker.rs

@@ -349,10 +349,15 @@ impl StructureChangeTracker {
             for key in order {
                 if let Some(col) = latest.get(&key) {
                     match build_alter_column(driver_id, key.0.as_deref(), &key.1, col) {
-                        Ok(sql) => out.push(sql),
-                        // NoChange means the post-edit state matches the
-                        // original — no statement to emit. Real errors
-                        // still propagate.
+                        // MySQL emits exactly one MODIFY COLUMN; the
+                        // Vec is always length 1. Postgres returns
+                        // up to three statements when type +
+                        // nullable + default all changed in the same
+                        // op; flatten them.
+                        Ok(stmts) => out.extend(stmts),
+                        // NoChange means the post-edit state matches
+                        // the original — no statements to emit. Real
+                        // errors still propagate.
                         Err(BuildDdlError::NoChange) => {}
                         Err(e) => return Err(e),
                     }
@@ -362,7 +367,7 @@ impl StructureChangeTracker {
             for op in &self.ops {
                 if let StructureOp::AlterColumn { schema, table, column } = op {
                     match build_alter_column(driver_id, schema.as_deref(), table, column) {
-                        Ok(sql) => out.push(sql),
+                        Ok(stmts) => out.extend(stmts),
                         Err(BuildDdlError::NoChange) => {}
                         Err(e) => return Err(e),
                     }

linux/crates/app/src/ui/app/structure.rs

@@ -326,6 +326,12 @@ impl App {
                         }
                     }
                     if wrap_tx && let Err(e) = conn.execute("COMMIT").await {
+                        // COMMIT failure leaves the connection in an
+                        // open transaction. Fire ROLLBACK so the
+                        // pooled connection is reusable; ignore its
+                        // result because the user already has the
+                        // commit-failure to surface.
+                        let _ = conn.execute("ROLLBACK").await;
                         sender_for_cmd.input(AppMsg::StructureSaveFailed(tab_id, format!("{e}")));
                         return;
                     }
@@ -427,6 +433,14 @@ impl App {
             self.in_flight_saves.set(self.in_flight_saves.get() - 1);
         }
         self.structure_saves_in_flight.borrow_mut().remove(&tab_id);
+        // Match the Browse-side `SaveFailedForTab` handler: a save
+        // that started from the close-with-pending dialog left this
+        // tab in `close_after_save`, and the window-close-after-save
+        // intent in `close_window_after_save`. Both must be cleared
+        // on failure or the next unrelated SaveCompleted on another
+        // tab will spuriously close the window.
+        self.close_after_save.borrow_mut().remove(&tab_id);
+        self.close_window_after_save.set(false);
         if let Some(controller) = self
             .workspace_tabs
             .borrow()

linux/crates/app/src/ui/app/workspace_tabs.rs

@@ -702,6 +702,13 @@ impl App {
         let Some(removed) = removed else {
             return;
         };
+        // Drop any close-after-save / window-close-after-save intent
+        // pinned to this tab. `close_tabs_for_table` (Drop Table)
+        // calls into here directly without going through the per-tab
+        // close prompt, so a stale entry would otherwise live on
+        // forever and cause a future unrelated SaveCompleted to
+        // spuriously close the window.
+        self.close_after_save.borrow_mut().remove(&id);
         match &removed {
             WorkspaceTab::Editor(slot) => {
                 let _ = slot.controller.sender().send(SqlEditorInput::Cancel);

linux/crates/app/src/ui/browse_tab.rs

@@ -1567,12 +1567,17 @@ impl SimpleComponent for BrowseTab {
                 if self.current_columns.is_empty() {
                     return;
                 }
-                let Some(snapshot) = self.current_result.as_ref() else {
-                    return;
-                };
-                let Some(source_row) = snapshot.rows.get(row_position as usize) else {
+                // Read the source row through the live RowObject at
+                // `row_position` in the FilterListModel — NOT by
+                // indexing `current_result.rows` directly. The grid's
+                // row_position reflects the user's current sort + any
+                // active search filter; raw `rows` is fetch order. A
+                // sort or filter would otherwise hand us the wrong
+                // row's cells.
+                let Some(source) = self.row_object_at(row_position) else {
                     return;
                 };
+                let source_cells = source.cells_clone();
                 // Clone source values; blank columns whose value is
                 // owned by the database (PK, identity / serial,
                 // generated). The duplicate is meant to be a *new*
@@ -1586,7 +1591,7 @@ impl SimpleComponent for BrowseTab {
                         if col.primary_key || col.is_auto_increment || col.is_generated {
                             Value::Null
                         } else {
-                            source_row.get(i).cloned().unwrap_or(Value::Null)
+                            source_cells.get(i).cloned().unwrap_or(Value::Null)
                         }
                     })
                     .collect();
@@ -2124,7 +2129,12 @@ enum TypeKind {
 /// before bare `timestamp`, and `tinyint(1)` (MySQL bool) must be
 /// matched before generic `tinyint` / `int` patterns.
 fn classify_type(dt: &str) -> TypeKind {
-    if matches!(dt, "bool" | "boolean" | "bit" | "tinyint(1)") {
+    // Postgres `format_type()` returns "bit(1)" for length-1 BIT
+    // columns (not the bare "bit" the original guard expected).
+    // Both forms classify as Bool so the cell renders as a checkbox
+    // rather than a text editor that rejects "true"/"false" with
+    // "Invalid integer".
+    if matches!(dt, "bool" | "boolean" | "bit" | "bit(1)" | "tinyint(1)") {
         return TypeKind::Bool;
     }
     if dt.contains("uuid") {

linux/crates/app/src/ui/editor.rs

@@ -333,10 +333,29 @@ impl SimpleComponent for SqlEditor {
                                 } else {
                                     Box::pin(std::future::pending::<()>())
                                 };
+                            // The cancel token is the editor's own
+                            // signal channel — the driver does not
+                            // subscribe to it (sqlx has no future-
+                            // drop cancellation hook for Postgres /
+                            // MySQL). When the timeout wins, we
+                            // *also* fire `token.cancel()` so any
+                            // outer logic (pool shutdown, connection
+                            // monitor) sees the same "this query is
+                            // abandoned" signal as a manual Cancel,
+                            // and the future drops on the next poll.
+                            // Even with this, the underlying driver
+                            // call may keep running on the server
+                            // until the network layer notices the
+                            // dropped read; users on long timeouts
+                            // should restart the connection.
+                            let token_for_timeout = token.clone();
                             let msg = tokio::select! {
                                 biased;
                                 _ = token.cancelled() => SqlEditorInput::ShowCancelled,
-                                _ = timeout => SqlEditorInput::ShowTimedOut(timeout_secs),
+                                _ = timeout => {
+                                    token_for_timeout.cancel();
+                                    SqlEditorInput::ShowTimedOut(timeout_secs)
+                                }
                                 outcomes = run_statements(conn, statements) => {
                                     let total_ms: u128 = outcomes.iter().map(|o| o.elapsed_ms).sum();
                                     let n_ok = outcomes
@@ -447,7 +466,7 @@ impl SimpleComponent for SqlEditor {
                 let page = adw::StatusPage::builder()
                     .title(crate::tr!("Query timed out"))
                     .description(&reason)
-                    .icon_name("appointment-soon-symbolic")
+                    .icon_name("dialog-warning-symbolic")
                     .vexpand(true)
                     .build();
                 self.results_holder.append(&page);

linux/crates/app/src/ui/structure_tab.rs

@@ -21,7 +21,7 @@
 //! sets the SourceView buffer text. The Save button reads
 //! `materialize` again and dispatches `ExecuteTransaction` to App.
 
-use std::cell::RefCell;
+use std::cell::{Cell, RefCell};
 use std::rc::Rc;
 
 use relm4::adw::prelude::*;
@@ -175,8 +175,11 @@ pub struct StructureTab {
     last_dirty: Rc<RefCell<bool>>,
     /// Suppress reentrant rebuilds: setting Entry text triggers
     /// changed-signal echoes that would push duplicate RenameTable
-    /// ops onto the tracker.
-    suppress_emit: Rc<RefCell<bool>>,
+    /// ops onto the tracker. `Cell` (not `RefCell`) because GTK can
+    /// fire those signals re-entrantly during `clear_box` while
+    /// another `borrow_mut` is still live on the stack — a
+    /// `RefCell::borrow` racing it would panic.
+    suppress_emit: Rc<Cell<bool>>,
     /// Monotonic counter for the "Add Column" placeholder name. Using
     /// `vec.len() + 1` produced duplicate `column_1` after the user
     /// removed a column; a forever-incrementing counter avoids the
@@ -287,7 +290,7 @@ impl StructureTab {
         // mode reload would land 7 columns × ~3 fields ≈ 19 spurious
         // pending changes. We re-enable emit on the next idle tick so
         // legitimate user input afterwards flows through.
-        *self.suppress_emit.borrow_mut() = true;
+        self.suppress_emit.set(true);
         clear_box(&self.columns_box);
         let driver_id = self.driver_id.clone();
         let list = boxed_list();
@@ -308,7 +311,7 @@ impl StructureTab {
         self.columns_box.append(&wrap_button_in_row(add_button));
         let suppress = self.suppress_emit.clone();
         relm4::gtk::glib::idle_add_local_once(move || {
-            *suppress.borrow_mut() = false;
+            suppress.set(false);
         });
     }
 
@@ -435,7 +438,7 @@ fn build_column_row(
     col: &DraftColumn,
     driver_id: &str,
     sender: ComponentSender<StructureTab>,
-    suppress_emit: Rc<RefCell<bool>>,
+    suppress_emit: Rc<Cell<bool>>,
 ) -> gtk::Widget {
     let row = gtk::Box::builder()
         .orientation(gtk::Orientation::Horizontal)
@@ -459,7 +462,7 @@ fn build_column_row(
     let sender_for_name = sender.clone();
     let suppress_for_name = suppress_emit.clone();
     name_entry.connect_changed(move |e| {
-        if *suppress_for_name.borrow() {
+        if suppress_for_name.get() {
             return;
         }
         sender_for_name.input(StructureTabInput::ColumnEdited {
@@ -481,7 +484,7 @@ fn build_column_row(
         let sender_for_type = sender.clone();
         let suppress_for_type = suppress_emit.clone();
         entry.connect_changed(move |e| {
-            if *suppress_for_type.borrow() {
+            if suppress_for_type.get() {
                 return;
             }
             sender_for_type.input(StructureTabInput::ColumnEdited {
@@ -509,7 +512,7 @@ fn build_column_row(
     let sender_for_null = sender.clone();
     let suppress_for_null = suppress_emit.clone();
     nullable_check.connect_toggled(move |c| {
-        if *suppress_for_null.borrow() {
+        if suppress_for_null.get() {
             return;
         }
         sender_for_null.input(StructureTabInput::ColumnEdited {
@@ -532,7 +535,7 @@ fn build_column_row(
     let sender_for_default = sender.clone();
     let suppress_for_default = suppress_emit.clone();
     default_entry.connect_changed(move |e| {
-        if *suppress_for_default.borrow() {
+        if suppress_for_default.get() {
             return;
         }
         let text = e.text().to_string();
@@ -553,7 +556,7 @@ fn build_column_row(
     let sender_for_pk = sender.clone();
     let suppress_for_pk = suppress_emit.clone();
     pk_check.connect_toggled(move |c| {
-        if *suppress_for_pk.borrow() {
+        if suppress_for_pk.get() {
             return;
         }
         sender_for_pk.input(StructureTabInput::ColumnEdited {
@@ -572,7 +575,7 @@ fn build_column_row(
     let sender_for_auto = sender.clone();
     let suppress_for_auto = suppress_emit;
     auto_check.connect_toggled(move |c| {
-        if *suppress_for_auto.borrow() {
+        if suppress_for_auto.get() {
             return;
         }
         sender_for_auto.input(StructureTabInput::ColumnEdited {
@@ -1193,11 +1196,11 @@ impl SimpleComponent for StructureTab {
 
         // Wire the table-name entry to push RenameTable / propagate to
         // the model.
-        let suppress_emit = Rc::new(RefCell::new(false));
+        let suppress_emit = Rc::new(Cell::new(false));
         let sender_for_name = sender.clone();
         let suppress_for_name = suppress_emit.clone();
         name_entry.connect_changed(move |e| {
-            if *suppress_for_name.borrow() {
+            if suppress_for_name.get() {
                 return;
             }
             sender_for_name.input(StructureTabInput::TableNameEdited(e.text().to_string()));
@@ -1648,9 +1651,9 @@ impl SimpleComponent for StructureTab {
                 if let Some(name) = new_table_name {
                     *self.mode.borrow_mut() = StructureMode::Edit;
                     *self.table_name.borrow_mut() = name.clone();
-                    *self.suppress_emit.borrow_mut() = true;
+                    self.suppress_emit.set(true);
                     self.name_entry.set_text(&name);
-                    *self.suppress_emit.borrow_mut() = false;
+                    self.suppress_emit.set(false);
                     self.drop_button.set_visible(true);
                     // New mode → Edit mode: the prominent name row at
                     // the top of the editor was a New-mode-only