Stop the tabular workspace connection pool from leaking read-only state

src/Primitives/CrestApps.Core.AI.Documents/Handlers/TabularWorkspaceDocumentEventHandler.cs

@@ -1,7 +1,6 @@
 using CrestApps.Core.AI.Documents.Models;
 using CrestApps.Core.AI.Documents.Tabular;
 using CrestApps.Core.AI.Models;
-using Microsoft.Data.Sqlite;
 using Microsoft.Extensions.Logging;
 using Microsoft.Extensions.Options;
 
@@ -46,22 +45,18 @@ public async Task RemovedAsync(AIChatDocumentRemoveContext context, Cancellation
     /// </summary>
     private void TryDropDocumentTable(string referenceType, string referenceId, string documentId)
     {
-        if (string.IsNullOrEmpty(_basePath) || string.IsNullOrEmpty(referenceType) || string.IsNullOrEmpty(referenceId))
-        {
-            return;
-        }
+        var databasePath = TabularWorkspaceDatabase.GetDatabasePath(_basePath, referenceType, referenceId);
 
-        var databasePath = Path.Combine(_basePath, "documents", referenceType, referenceId, "data", "tabular.db");
-
-        if (!File.Exists(databasePath))
+        if (databasePath is null || !File.Exists(databasePath))
         {
             return;
         }
 
         try
         {
-            using var connection = new SqliteConnection($"Data Source={databasePath}");
-            connection.Open();
+            // The workspace keeps its connection read-only between write windows, so the connection is
+            // opened through the shared helper that turns query_only back off before anything writes.
+            using var connection = TabularWorkspaceDatabase.Open(databasePath);
 
             // A single document can produce multiple tables (one per worksheet), so drop them all.
             var tableNames = new List<string>();
@@ -115,18 +110,19 @@ SELECT COUNT(*) FROM "_workspace_meta"
         }
         catch (Exception ex)
         {
-            if (_logger.IsEnabled(LogLevel.Debug))
-            {
-                _logger.LogDebug(ex, "Failed to drop tables for document '{DocumentId}' from workspace database.", documentId);
-            }
+            // The document was removed from the conversation but its data is still in the workspace
+            // database, so the model can keep querying it. That is worth surfacing rather than hiding
+            // behind a debug-level log.
+            _logger.LogWarning(ex, "Failed to drop tables for document '{DocumentId}' from workspace database.", documentId);
         }
     }
 
     private void TryDeleteDatabaseFiles(string databasePath)
     {
-        TryDeleteFile(databasePath);
-        TryDeleteFile(databasePath + "-wal");
-        TryDeleteFile(databasePath + "-shm");
+        foreach (var path in TabularWorkspaceDatabase.GetDatabaseFilePaths(databasePath))
+        {
+            TryDeleteFile(path);
+        }
     }
 
     private void TryDeleteFile(string path)

src/Primitives/CrestApps.Core.AI.Documents/Handlers/TabularWorkspaceHistoryClearedHandler.cs

@@ -1,4 +1,5 @@
 using CrestApps.Core.AI.Chat;
+using CrestApps.Core.AI.Documents.Tabular;
 using CrestApps.Core.AI.Models;
 using Microsoft.Extensions.Logging;
 using Microsoft.Extensions.Options;
@@ -44,10 +45,17 @@ public Task HistoryClearedAsync(
             return Task.CompletedTask;
         }
 
-        var databasePath = Path.Combine(_basePath, "documents", AIReferenceTypes.Document.ChatInteraction, interaction.ItemId, "data", "tabular.db");
-        TryDeleteFile(databasePath);
-        TryDeleteFile(databasePath + "-wal");
-        TryDeleteFile(databasePath + "-shm");
+        var databasePath = TabularWorkspaceDatabase.GetDatabasePath(_basePath, AIReferenceTypes.Document.ChatInteraction, interaction.ItemId);
+
+        if (databasePath is null)
+        {
+            return Task.CompletedTask;
+        }
+
+        foreach (var path in TabularWorkspaceDatabase.GetDatabaseFilePaths(databasePath))
+        {
+            TryDeleteFile(path);
+        }
 
         return Task.CompletedTask;
     }

src/Primitives/CrestApps.Core.AI.Documents/Services/DefaultConversationDocumentCleanupService.cs

@@ -1,4 +1,3 @@
-using System.Text.RegularExpressions;
 using CrestApps.Core.AI.Documents.Generation;
 using CrestApps.Core.AI.Documents.Tabular;
 using CrestApps.Core.AI.Models;
@@ -17,8 +16,6 @@ namespace CrestApps.Core.AI.Documents.Services;
 /// </summary>
 public sealed class DefaultConversationDocumentCleanupService : IConversationDocumentCleanupService
 {
-    private static readonly Regex _safePathSegmentExpression = new("^[a-zA-Z0-9._-]+$", RegexOptions.Compiled);
-
     private readonly IAIDocumentStore _documentStore;
     private readonly IAIDocumentChunkStore _chunkStore;
     private readonly IDocumentFileStore _fileStore;
@@ -64,21 +61,19 @@ public async Task CleanupAsync(string referenceId, string referenceType, Cancell
 
         var documents = await _documentStore.GetDocumentsAsync(referenceId, referenceType);
 
-        if (documents.Count == 0)
-        {
-            return;
-        }
-
         foreach (var document in documents)
         {
             cancellationToken.ThrowIfCancellationRequested();
 
             await DeleteDocumentAsync(document, cancellationToken);
         }
 
+        // Runs even when the conversation has no documents left. The workspace database is a separate
+        // file that outlives the document rows, so a conversation whose spreadsheets were each removed
+        // one by one still has a database to delete, and returning early would orphan it forever.
         TryDeleteTabularDatabase(referenceType, referenceId);
 
-        if (_logger.IsEnabled(LogLevel.Debug))
+        if (documents.Count > 0 && _logger.IsEnabled(LogLevel.Debug))
         {
             _logger.LogDebug(
                 "Removed {DocumentCount} document(s) for conversation '{ReferenceId}' of type '{ReferenceType}'.",
@@ -142,15 +137,17 @@ private async Task DeleteDocumentAsync(AIDocument document, CancellationToken ca
 
     private void TryDeleteTabularDatabase(string referenceType, string referenceId)
     {
-        var databasePath = BuildTabularDatabaseStoragePath(referenceType, referenceId);
+        var databasePath = TabularWorkspaceDatabase.GetStorageRelativePath(referenceType, referenceId);
+
         if (databasePath is null)
         {
             return;
         }
 
-        TryDeleteFile(databasePath);
-        TryDeleteFile(databasePath + "-wal");
-        TryDeleteFile(databasePath + "-shm");
+        foreach (var path in TabularWorkspaceDatabase.GetDatabaseFilePaths(databasePath))
+        {
+            TryDeleteFile(path);
+        }
     }
 
     private void TryDeleteFile(string path)
@@ -167,23 +164,4 @@ private void TryDeleteFile(string path)
             }
         }
     }
-
-    private static string BuildTabularDatabaseStoragePath(string referenceType, string referenceId)
-    {
-        if (!IsSafePathSegment(referenceType) || !IsSafePathSegment(referenceId))
-        {
-            return null;
-        }
-
-        return Path.Combine("documents", referenceType, referenceId, "data", "tabular.db")
-            .Replace(Path.DirectorySeparatorChar, '/')
-            .Replace(Path.AltDirectorySeparatorChar, '/');
-    }
-
-    private static bool IsSafePathSegment(string value)
-    {
-        return !string.IsNullOrWhiteSpace(value)
-            && value is not "." and not ".."
-            && _safePathSegmentExpression.IsMatch(value);
-    }
 }

src/Primitives/CrestApps.Core.AI.Documents/Tabular/TabularToolContext.cs

@@ -251,12 +251,7 @@ private static string ResolveDatabasePath(IServiceProvider services, string refe
 
         var fileStoreOptions = services.GetRequiredService<IOptions<DocumentFileSystemFileStoreOptions>>().Value;
 
-        if (string.IsNullOrEmpty(fileStoreOptions.BasePath))
-        {
-            return null;
-        }
-
-        return Path.Combine(fileStoreOptions.BasePath, "documents", referenceType, referenceId, "data", "tabular.db");
+        return TabularWorkspaceDatabase.GetDatabasePath(fileStoreOptions.BasePath, referenceType, referenceId);
     }
 
     private static AIChatSession ResolveSession()

src/Primitives/CrestApps.Core.AI.Documents/Tabular/TabularWorkspace.cs

@@ -120,6 +120,7 @@ public async Task<IReadOnlyList<TabularTableInfo>> EnsureReadyAsync(
 
             try
             {
+                RemoveTablesForDetachedDocuments(documents);
                 await SynchronizeTablesAsync(documents, artifactLoader, workspaceImporter, cancellationToken);
             }
             finally
@@ -559,16 +560,7 @@ private void EnsureLoaded()
     /// <param name="connection">The connection to toggle.</param>
     /// <param name="writable">When <see langword="true"/>, writes are allowed; otherwise they are blocked.</param>
     private static void SetWritable(SqliteConnection connection, bool writable)
-    {
-        if (connection is null)
-        {
-            return;
-        }
-
-        using var command = connection.CreateCommand();
-        command.CommandText = writable ? "PRAGMA query_only = OFF" : "PRAGMA query_only = ON";
-        command.ExecuteNonQuery();
-    }
+        => TabularWorkspaceDatabase.SetWritable(connection, writable);
 
     private async Task SynchronizeTablesAsync(
         IReadOnlyList<TabularDocumentRef> documents,
@@ -655,6 +647,80 @@ private async Task SynchronizeTablesAsync(
         }
     }
 
+    /// <summary>
+    /// Drops every table whose document is no longer attached to the conversation.
+    /// </summary>
+    /// <remarks>
+    /// The workspace database outlives a single request, so a document removed from the conversation
+    /// leaves its tables behind and the model can keep querying data the user believes is gone. The
+    /// removal handler drops them eagerly; this pass is what makes the workspace self-correcting when
+    /// that never ran, failed, or the document disappeared by another route. The caller supplies the
+    /// complete document set for the scope, so anything else in the database is detached by
+    /// definition. Must run inside a write window.
+    /// </remarks>
+    /// <param name="documents">The documents currently attached to the conversation.</param>
+    private void RemoveTablesForDetachedDocuments(IReadOnlyList<TabularDocumentRef> documents)
+    {
+        if (_tables.Count == 0)
+        {
+            return;
+        }
+
+        var attached = new HashSet<string>(StringComparer.Ordinal);
+
+        foreach (var document in documents)
+        {
+            attached.Add(document.DocumentId);
+        }
+
+        List<LoadedTable> detached = null;
+
+        foreach (var table in _tables.Values)
+        {
+            if (!attached.Contains(table.DocumentId))
+            {
+                (detached ??= []).Add(table);
+            }
+        }
+
+        if (detached is null)
+        {
+            return;
+        }
+
+        foreach (var table in detached)
+        {
+            DropTable(table.TableName);
+            DeleteMetadataEntry(table.TableName);
+            _tables.Remove(table.TableName);
+
+            if (_logger.IsEnabled(LogLevel.Debug))
+            {
+                _logger.LogDebug(
+                    "Dropped tabular table '{TableName}' because document '{DocumentId}' is no longer attached to the conversation.",
+                    table.TableName,
+                    table.DocumentId);
+            }
+        }
+    }
+
+    private void DropTable(string tableName)
+    {
+        using var command = _connection.CreateCommand();
+        command.CommandText = $"DROP TABLE IF EXISTS \"{tableName.Replace("\"", "\"\"", StringComparison.Ordinal)}\"";
+        command.ExecuteNonQuery();
+    }
+
+    private void DeleteMetadataEntry(string tableName)
+    {
+        using var command = _connection.CreateCommand();
+        command.CommandText = $"""
+            DELETE FROM "{MetadataTableName}" WHERE table_name = $tableName
+            """;
+        command.Parameters.AddWithValue("$tableName", tableName);
+        command.ExecuteNonQuery();
+    }
+
     private bool IsDocumentLoaded(string documentId)
     {
         foreach (var table in _tables.Values)
@@ -709,26 +775,19 @@ private static string AllocateTableName(
 
     private SqliteConnection OpenConnection()
     {
-        string connectionString;
-
-        if (string.IsNullOrEmpty(_databasePath))
-        {
-            connectionString = "Data Source=:memory:";
-        }
-        else
+        if (!string.IsNullOrEmpty(_databasePath))
         {
             var directory = Path.GetDirectoryName(_databasePath);
 
             if (!string.IsNullOrEmpty(directory))
             {
                 Directory.CreateDirectory(directory);
             }
-
-            connectionString = $"Data Source={_databasePath}";
         }
 
-        var connection = new SqliteConnection(connectionString);
-        connection.Open();
+        // Opens the connection with writes enabled so the journal-mode and metadata statements below
+        // succeed; the write window is closed again once the database is ready.
+        var connection = TabularWorkspaceDatabase.Open(_databasePath);
         EnableDoubleQuotedStringLiterals(connection);
 
         if (_logger.IsEnabled(LogLevel.Debug))