fix(structure): confirm a Structure save once, at the execution gate or the rebuild review, and stay quiet after Cancel

CHANGELOG.md

@@ -209,6 +209,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - **New Trigger** offered on a materialized view.
 - Structure tab refusing every save on a SQLite, libSQL or Cloudflare D1 table with a column that has no declared type.
 - Structure tab refusing to save a renamed or dropped primary key column.
+- Structure saves that could lose data, and table rebuilds, asking twice for one confirmation, and showing an error after Cancel.
 - Compressed dump named `.GZ` rather than `.gz` reaching the parser still compressed.
 - **SQL** offered as an import format on MongoDB.
 - **Save** permanently dim on a Custom provider for an OpenAI-compatible server that wants no API key.

TablePro/Core/Coordinators/QueryExecutionCoordinator.swift

@@ -86,7 +86,7 @@ final class QueryExecutionCoordinator: ObservableObject {
                 case .needsBatchDriver:
                     break
                 }
-            case .denied(let reason):
+            case .denied(let reason, _):
                 parent.tabManager.mutate(at: index) { $0.execution.errorMessage = reason }
             }
         }
@@ -128,7 +128,7 @@ final class QueryExecutionCoordinator: ObservableObject {
             switch await ExecutionGateProvider.shared.authorize(request) {
             case .authorized:
                 executeParameterizedAfterSafeMode(route, parameters: parameters, bypassRowLimit: bypassRowLimit)
-            case .denied(let reason):
+            case .denied(let reason, _):
                 parent.tabManager.mutate(tabId: tabId) { $0.execution.errorMessage = reason }
             }
         }

TablePro/Core/Coordinators/RowEditingCoordinator+SaveChanges.swift

@@ -138,7 +138,7 @@ extension RowEditingCoordinator {
                     pendingDeletes: &dels,
                     tableOperationOptions: &opts
                 )
-            case .denied(let reason):
+            case .denied(let reason, _):
                 if hasPendingTableOps {
                     restorePendingTableOperations(
                         connectionId: connId,

TablePro/Core/Database/DatabaseManager+Schema.swift

@@ -21,33 +21,23 @@ extension DatabaseManager {
     /// Authorization sits outside the scoped block: it awaits a confirmation sheet and Touch ID,
     /// and holding the connection's driver gate across a human prompt would freeze every other
     /// tab on that connection.
+    ///
+    /// The gate's sheet is the only confirmation a save gets. A refusal throws
+    /// `ExecutionGateError.denied` and a Cancel throws `.cancelledByUser`, so the caller can keep
+    /// the edits staged and stay quiet about a choice the user just made.
     func executeSchemaChanges(
         _ statements: [SchemaStatement],
         databaseType: DatabaseType,
         scope: DatabaseScope,
-        table: String
+        table: String,
+        gate: any ExecutionGate = ExecutionGateProvider.shared
     ) async throws {
         let route = schemaChangeRoute(for: scope)
 
-        let combinedSQL = statements.map(\.sql).joined(separator: "\n")
-        let schemaKind: OperationKind =
-            QueryClassifier.classifyTier(combinedSQL, databaseType: databaseType) == .destructive
-            ? .destructiveQuery : .schemaMutation
-        let authorization = await ExecutionGateProvider.shared.authorize(
-            OperationRequest(
-                connectionId: scope.connectionId,
-                databaseType: databaseType,
-                sql: combinedSQL,
-                kind: schemaKind,
-                caller: .userInterface,
-                capabilities: .interactiveUser,
-                operationDescription: String(localized: "Apply Schema Changes")
-            )
-        )
-        guard case .authorized = authorization else {
-            throw DatabaseError.queryFailed(
-                authorization.deniedReason ?? String(localized: "Schema change was not authorized")
-            )
+        let request = Self.schemaChangeAuthorizationRequest(statements, databaseType: databaseType, scope: scope)
+        let schemaKind = request.kind
+        if let denial = await gate.authorize(request).denialError {
+            throw denial
         }
 
         let executionTimes: [TimeInterval]
@@ -113,6 +103,38 @@ extension DatabaseManager {
         )
     }
 
+    /// What the gate is asked before a save runs, built apart from the run so a test can put it to
+    /// the real gate at every Safe Mode level.
+    nonisolated static func schemaChangeAuthorizationRequest(
+        _ statements: [SchemaStatement],
+        databaseType: DatabaseType,
+        scope: DatabaseScope
+    ) -> OperationRequest {
+        let combinedSQL = statements.map(\.sql).joined(separator: "\n")
+        return OperationRequest(
+            connectionId: scope.connectionId,
+            databaseType: databaseType,
+            sql: combinedSQL,
+            kind: schemaOperationKind(for: statements, combinedSQL: combinedSQL, databaseType: databaseType),
+            caller: .userInterface,
+            capabilities: .interactiveUser,
+            operationDescription: String(localized: "Apply Schema Changes")
+        )
+    }
+
+    /// Destructive when the text reads that way or when any statement was generated as one. The
+    /// text alone cannot see that `ALTER COLUMN .. TYPE`, `MODIFY COLUMN .. NOT NULL` or an added
+    /// `CHECK` can lose or refuse existing rows, and a destructive kind is confirmed at every
+    /// Safe Mode level, Silent included.
+    nonisolated static func schemaOperationKind(
+        for statements: [SchemaStatement],
+        combinedSQL: String,
+        databaseType: DatabaseType
+    ) -> OperationKind {
+        let destructiveText = QueryClassifier.classifyTier(combinedSQL, databaseType: databaseType) == .destructive
+        return destructiveText || statements.contains(where: \.isDestructive) ? .destructiveQuery : .schemaMutation
+    }
+
     /// Run a Create Table draft's statements, on the same isolated route and in the same shape as
     /// `executeSchemaChanges`.
     ///

TablePro/Core/MCP/MCPAuthPolicy.swift

@@ -330,7 +330,7 @@ public actor MCPAuthPolicy {
                 operationDescription: Self.operationDescription(for: operationLabel)
             )
         )
-        if case .denied(let reason) = decision {
+        if case .denied(let reason, _) = decision {
             throw DatabaseAccessError.forbidden(reason)
         }
     }

TablePro/Core/SchemaTracking/SchemaStatementGenerator.swift

@@ -61,7 +61,11 @@ struct SchemaStatementGenerator {
             }
             for stmt in stmts {
                 let sql = stmt.sql.hasSuffix(";") ? stmt.sql : stmt.sql + ";"
-                statements.append(SchemaStatement(sql: sql, description: stmt.description, isDestructive: stmt.isDestructive))
+                statements.append(SchemaStatement(
+                    sql: sql,
+                    description: stmt.description,
+                    isDestructive: stmt.isDestructive || change.requiresDataMigration
+                ))
             }
         }
 

TablePro/Core/Services/Execution/DefaultExecutionGate.swift

@@ -90,7 +90,7 @@ internal actor DefaultExecutionGate: ExecutionGate {
                 )
             )
             guard confirmed else {
-                return .denied(reason: String(localized: "Operation cancelled by user"))
+                return .denied(reason: String(localized: "Operation cancelled by user"), cause: .cancelledByUser)
             }
         }
 

TablePro/Core/Services/Execution/ExternalStatementGate.swift

@@ -168,7 +168,7 @@ internal enum ExternalStatementGate {
                 operationDescription: operationDescription
             )
         )
-        if case .denied(let reason) = decision {
+        if case .denied(let reason, _) = decision {
             throw ExternalStatementGateError.denied(reason)
         }
     }

TablePro/Core/Services/Execution/OperationDecision.swift

@@ -21,9 +21,17 @@ internal struct OperationReceipt: Sendable, Equatable {
     }
 }
 
+/// Why the gate said no. Only the gate knows whether the person at the keyboard answered Cancel or
+/// a rule refused them, and a caller needs that to stay quiet after a Cancel rather than report the
+/// user's own choice back to them as a failure.
+internal enum OperationDenialCause: Sendable, Equatable {
+    case policy
+    case cancelledByUser
+}
+
 internal enum OperationDecision: Sendable {
     case authorized(OperationReceipt)
-    case denied(reason: String)
+    case denied(reason: String, cause: OperationDenialCause = .policy)
 }
 
 internal extension OperationDecision {
@@ -35,19 +43,31 @@ internal extension OperationDecision {
     }
 
     var deniedReason: String? {
-        if case .denied(let reason) = self {
+        if case .denied(let reason, _) = self {
             return reason
         }
         return nil
     }
+
+    /// What a caller that throws its denial raises, with a Cancel kept apart from a refusal.
+    var denialError: ExecutionGateError? {
+        guard case .denied(let reason, let cause) = self else { return nil }
+        switch cause {
+        case .policy:
+            return .denied(reason)
+        case .cancelledByUser:
+            return .cancelledByUser(reason)
+        }
+    }
 }
 
 internal enum ExecutionGateError: LocalizedError {
     case denied(String)
+    case cancelledByUser(String)
 
     var errorDescription: String? {
         switch self {
-        case .denied(let reason):
+        case .denied(let reason), .cancelledByUser(let reason):
             return reason
         }
     }

TablePro/Core/Services/Execution/Providers/OperationConfirming.swift

@@ -34,8 +34,14 @@ internal enum OperationConfirmationPrompt {
     }
 
     internal static func subtitle(of request: OperationConfirmationRequest) -> String {
-        let connection = request.connectionName?.trimmingCharacters(in: .whitespacesAndNewlines)
-        guard let client = clientName(for: request.caller) else {
+        subtitle(connectionName: request.connectionName, caller: request.caller)
+    }
+
+    /// The line under a confirmation's heading, for a sheet that confirms without going through
+    /// the gate's own prompt, the table rebuild review among them.
+    internal static func subtitle(connectionName: String?, caller: OperationCaller) -> String {
+        let connection = connectionName?.trimmingCharacters(in: .whitespacesAndNewlines)
+        guard let client = clientName(for: caller) else {
             guard let connection, !connection.isEmpty else {
                 return String(localized: "Review this before it runs.")
             }
@@ -47,9 +53,12 @@ internal enum OperationConfirmationPrompt {
         return String(format: String(localized: "%1$@ wants to run this on '%2$@'."), client, connection)
     }
 
+    internal static var destructiveDataWarning: String {
+        String(localized: "This may permanently modify or delete data and cannot be undone.")
+    }
+
     internal static func destructiveWarning(of request: OperationConfirmationRequest) -> String? {
-        guard request.isDestructive else { return nil }
-        return String(localized: "This may permanently modify or delete data and cannot be undone.")
+        request.isDestructive ? destructiveDataWarning : nil
     }
 
     /// A rename has no statement to show: the driver builds it from the names, and two engines

TablePro/Models/Schema/TableRebuildReviewRequest.swift

@@ -17,6 +17,8 @@ struct TableRebuildReviewRequest: Identifiable {
         /// What the confirming button says, which is the only thing that differs between a reorder
         /// and a constraint change: both recreate the table, and the user is reading the same script.
         let title: String
+        /// The operation the gate is told it is authorizing, which the sheet is titled with.
+        let operationDescription: String
         let perform: () async -> Void
     }
 
@@ -33,8 +35,12 @@ struct TableRebuildReviewRequest: Identifiable {
 
     let action: Action?
 
+    /// A rebuild drops the table it copied, so a sheet that can run it warns the way the gate's own
+    /// sheet does for a destructive statement, ahead of what the rebuild cannot carry over.
     var warning: String? {
-        plan.caveats.isEmpty ? nil : plan.caveats.joined(separator: " ")
+        let dataWarning = runnableAction.map { _ in OperationConfirmationPrompt.destructiveDataWarning }
+        let lines = [dataWarning].compactMap { $0 } + plan.caveats
+        return lines.isEmpty ? nil : lines.joined(separator: " ")
     }
 
     var isRunnable: Bool { plan.isRunnable }
@@ -44,4 +50,16 @@ struct TableRebuildReviewRequest: Identifiable {
     }
 
     var scriptStatements: [String] { plan.scriptStatements }
+
+    /// A sheet that can run the script is that script's only confirmation: the run it starts tells
+    /// the gate so. It therefore reads as the gate's own sheet does, titled with the operation,
+    /// naming the connection, and showing the script uncut. A preview keeps the preview heading.
+    var confirmationTitle: String? { runnableAction?.operationDescription }
+
+    var showsStatementsVerbatim: Bool { runnableAction != nil }
+
+    func confirmationSubtitle(connectionName: String?) -> String? {
+        guard runnableAction != nil else { return nil }
+        return OperationConfirmationPrompt.subtitle(connectionName: connectionName, caller: .userInterface)
+    }
 }