From ed8d0cf8f489f6d24e07a1642fcbfaaf43bd8ef5 Mon Sep 17 00:00:00 2001 From: Mike Alhayek Date: Mon, 14 Sep 2026 10:12:22 -0700 Subject: [PATCH 1/3] Stop the tabular workspace connection pool from leaking read-only state Removing a spreadsheet from a chat logged "SQLite Error 8: 'attempt to write a readonly database'" and left the document's tables in place. TabularWorkspace keeps its connection in PRAGMA query_only = ON between its narrow write windows. query_only is connection state, and Microsoft.Data.Sqlite pools connections by connection string without resetting it, so the workspace returned a poisoned connection to the pool on dispose. TabularWorkspaceDocumentEventHandler then opened the same "Data Source={path}" string, received that connection, and its DROP TABLE failed while the preceding metadata SELECT succeeded. A pooled connection also keeps the native SQLite file handle open after Close(), so the follow-up delete of an emptied tabular.db threw IOException into a debug-only log and orphan database files accumulated. TabularWorkspaceHistoryClearedHandler had the same problem. TabularWorkspaceDatabase now owns how the workspace database is located and opened: file-backed connections disable pooling (the workspace holds a single long-lived connection per scope, so pooling bought nothing), Open() turns query_only off explicitly so no caller inherits a write state, and the path layout that was duplicated across four call sites lives in one place. A failed drop leaves a removed document queryable by the model, so it is now logged as a warning instead of at debug level. Co-Authored-By: Claude Opus 5 --- .../TabularWorkspaceDocumentEventHandler.cs | 30 ++- .../TabularWorkspaceHistoryClearedHandler.cs | 16 +- .../Tabular/TabularToolContext.cs | 7 +- .../Tabular/TabularWorkspace.cs | 26 +-- .../Tabular/TabularWorkspaceDatabase.cs | 102 ++++++++++ ...bularWorkspaceDocumentEventHandlerTests.cs | 184 +++++++++++++++++- 6 files changed, 315 insertions(+), 50 deletions(-) create mode 100644 src/Primitives/CrestApps.Core.AI.Documents/Tabular/TabularWorkspaceDatabase.cs diff --git a/src/Primitives/CrestApps.Core.AI.Documents/Handlers/TabularWorkspaceDocumentEventHandler.cs b/src/Primitives/CrestApps.Core.AI.Documents/Handlers/TabularWorkspaceDocumentEventHandler.cs index bd1ba1e7..fd3206ab 100644 --- a/src/Primitives/CrestApps.Core.AI.Documents/Handlers/TabularWorkspaceDocumentEventHandler.cs +++ b/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 /// 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(); @@ -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) diff --git a/src/Primitives/CrestApps.Core.AI.Documents/Handlers/TabularWorkspaceHistoryClearedHandler.cs b/src/Primitives/CrestApps.Core.AI.Documents/Handlers/TabularWorkspaceHistoryClearedHandler.cs index 9d236d58..b4799171 100644 --- a/src/Primitives/CrestApps.Core.AI.Documents/Handlers/TabularWorkspaceHistoryClearedHandler.cs +++ b/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; } diff --git a/src/Primitives/CrestApps.Core.AI.Documents/Tabular/TabularToolContext.cs b/src/Primitives/CrestApps.Core.AI.Documents/Tabular/TabularToolContext.cs index 1b951406..b6934040 100644 --- a/src/Primitives/CrestApps.Core.AI.Documents/Tabular/TabularToolContext.cs +++ b/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>().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() diff --git a/src/Primitives/CrestApps.Core.AI.Documents/Tabular/TabularWorkspace.cs b/src/Primitives/CrestApps.Core.AI.Documents/Tabular/TabularWorkspace.cs index 8f7a31b0..5c4a7c33 100644 --- a/src/Primitives/CrestApps.Core.AI.Documents/Tabular/TabularWorkspace.cs +++ b/src/Primitives/CrestApps.Core.AI.Documents/Tabular/TabularWorkspace.cs @@ -559,16 +559,7 @@ private void EnsureLoaded() /// The connection to toggle. /// When , writes are allowed; otherwise they are blocked. 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 documents, @@ -709,13 +700,7 @@ 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); @@ -723,12 +708,11 @@ private SqliteConnection OpenConnection() { 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)) diff --git a/src/Primitives/CrestApps.Core.AI.Documents/Tabular/TabularWorkspaceDatabase.cs b/src/Primitives/CrestApps.Core.AI.Documents/Tabular/TabularWorkspaceDatabase.cs new file mode 100644 index 00000000..3dd54e56 --- /dev/null +++ b/src/Primitives/CrestApps.Core.AI.Documents/Tabular/TabularWorkspaceDatabase.cs @@ -0,0 +1,102 @@ +using Microsoft.Data.Sqlite; + +namespace CrestApps.Core.AI.Documents.Tabular; + +/// +/// Centralizes how the file-backed tabular workspace database is located and opened so every caller +/// agrees on the path layout, the connection string, and the connection's initial write state. +/// +internal static class TabularWorkspaceDatabase +{ + private const string DatabaseFileName = "tabular.db"; + private const string DocumentsFolderName = "documents"; + private const string DataFolderName = "data"; + + /// + /// Builds the absolute path of the workspace database for a conversation scope. + /// + /// The document file store base path. + /// The reference type owning the workspace. + /// The reference identifier owning the workspace. + /// The database path, or when the scope is incomplete. + public static string GetDatabasePath(string basePath, string referenceType, string referenceId) + { + if (string.IsNullOrEmpty(basePath) || string.IsNullOrEmpty(referenceType) || string.IsNullOrEmpty(referenceId)) + { + return null; + } + + return Path.Combine(basePath, DocumentsFolderName, referenceType, referenceId, DataFolderName, DatabaseFileName); + } + + /// + /// Returns the database file and its write-ahead-log sidecars, which must be removed together. + /// + /// The database path. + /// The database path followed by its -wal and -shm sidecars. + public static string[] GetDatabaseFilePaths(string databasePath) + { + return [databasePath, databasePath + "-wal", databasePath + "-shm"]; + } + + /// + /// Builds the connection string for a workspace database. + /// + /// + /// Connection pooling is disabled for file-backed workspaces. A pooled connection keeps the + /// underlying SQLite handle open after , which both leaks + /// connection-level state such as the query_only pragma to the next caller that opens the + /// same file and holds a file lock that prevents the database from being deleted. The workspace + /// opens a single long-lived connection per scope, so pooling buys nothing here. + /// + /// The database path, or for an in-memory workspace. + /// The connection string. + public static string BuildConnectionString(string databasePath) + { + if (string.IsNullOrEmpty(databasePath)) + { + return "Data Source=:memory:"; + } + + return new SqliteConnectionStringBuilder + { + DataSource = databasePath, + Pooling = false, + }.ToString(); + } + + /// + /// Opens a connection to a workspace database and puts it into a known-writable state. + /// + /// The database path, or for an in-memory workspace. + /// The open connection. + public static SqliteConnection Open(string databasePath) + { + var connection = new SqliteConnection(BuildConnectionString(databasePath)); + connection.Open(); + + // Never inherit the write state from whatever opened this database last. The workspace runs + // most of its life with query_only turned on, so a caller that assumed the SQLite default + // would otherwise fail with "attempt to write a readonly database". + SetWritable(connection, true); + + return connection; + } + + /// + /// Toggles SQLite's connection-level query_only flag. + /// + /// The connection to toggle. + /// When , writes are allowed; otherwise they are blocked. + public 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(); + } +} diff --git a/tests/CrestApps.Core.Tests/Core/Documents/Tabular/TabularWorkspaceDocumentEventHandlerTests.cs b/tests/CrestApps.Core.Tests/Core/Documents/Tabular/TabularWorkspaceDocumentEventHandlerTests.cs index f10ed8dd..2c4ad9d5 100644 --- a/tests/CrestApps.Core.Tests/Core/Documents/Tabular/TabularWorkspaceDocumentEventHandlerTests.cs +++ b/tests/CrestApps.Core.Tests/Core/Documents/Tabular/TabularWorkspaceDocumentEventHandlerTests.cs @@ -4,6 +4,7 @@ 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.Abstractions; using Microsoft.Extensions.Options; using Moq; @@ -84,14 +85,193 @@ await handler.RemovedAsync(new AIChatDocumentRemoveContext Times.Once); } - private static TabularWorkspaceDocumentEventHandler CreateHandler(Mock artifactStore) + [Fact] + public async Task RemovedAsync_WhenTheDocumentOwnedTheLastTables_DeletesTheDatabaseFile() + { + var basePath = CreateTemporaryBasePath(); + + try + { + var databasePath = CreateWorkspaceDatabase(basePath, "session-1", "doc-1", "sales", "sales_q2"); + + var artifactStore = new Mock(); + var handler = CreateHandler(artifactStore, basePath); + + await handler.RemovedAsync(new AIChatDocumentRemoveContext + { + ReferenceId = "session-1", + ReferenceType = AIReferenceTypes.Document.ChatSession, + DocumentInfo = new ChatDocumentInfo + { + DocumentId = "doc-1", + FileName = "data.csv", + }, + }, TestContext.Current.CancellationToken); + + // The handler's own connection must release the file handle when it closes. A pooled + // connection keeps the SQLite handle open after Dispose and the delete silently fails. + Assert.False(File.Exists(databasePath)); + } + finally + { + TryDeleteDirectory(basePath); + } + } + + [Fact] + public async Task RemovedAsync_WhenAnEarlierConnectionLeftTheDatabaseReadOnly_StillDropsTheDocumentTables() + { + var basePath = CreateTemporaryBasePath(); + + try + { + var databasePath = CreateWorkspaceDatabase(basePath, "session-1", "doc-1", "sales"); + AddWorkspaceTable(databasePath, "doc-2", "budget"); + + // The workspace keeps its own connection in query_only mode between write windows. A + // pooled connection carries that pragma over to the next caller that opens the same file, + // so every write then fails with "attempt to write a readonly database". + LeaveDatabaseReadOnlyInThePool(databasePath); + + var artifactStore = new Mock(); + var handler = CreateHandler(artifactStore, basePath); + + await handler.RemovedAsync(new AIChatDocumentRemoveContext + { + ReferenceId = "session-1", + ReferenceType = AIReferenceTypes.Document.ChatSession, + DocumentInfo = new ChatDocumentInfo + { + DocumentId = "doc-1", + FileName = "data.csv", + }, + }, TestContext.Current.CancellationToken); + + Assert.True(File.Exists(databasePath)); + Assert.Equal(["budget"], GetUserTableNames(databasePath)); + } + finally + { + TryDeleteDirectory(basePath); + } + } + + private static string CreateTemporaryBasePath() + { + var basePath = Path.Combine(Path.GetTempPath(), "tabular-doc-event-tests", Path.GetRandomFileName()); + Directory.CreateDirectory(basePath); + + return basePath; + } + + private static string CreateWorkspaceDatabase(string basePath, string referenceId, string documentId, params string[] tableNames) + { + var databasePath = Path.Combine(basePath, "documents", AIReferenceTypes.Document.ChatSession, referenceId, "data", "tabular.db"); + Directory.CreateDirectory(Path.GetDirectoryName(databasePath)); + + using (var connection = OpenUnpooled(databasePath)) + { + using var command = connection.CreateCommand(); + command.CommandText = """ + CREATE TABLE IF NOT EXISTS "_workspace_meta" ( + "table_name" TEXT PRIMARY KEY, + "document_id" TEXT NOT NULL, + "worksheet_name" TEXT, + "file_name" TEXT NOT NULL, + "source_names_json" TEXT NOT NULL + ) + """; + command.ExecuteNonQuery(); + } + + foreach (var tableName in tableNames) + { + AddWorkspaceTable(databasePath, documentId, tableName); + } + + return databasePath; + } + + private static void AddWorkspaceTable(string databasePath, string documentId, string tableName) + { + using var connection = OpenUnpooled(databasePath); + + using (var createCommand = connection.CreateCommand()) + { + createCommand.CommandText = $"CREATE TABLE \"{tableName}\" (\"value\" TEXT)"; + createCommand.ExecuteNonQuery(); + } + + using var insertCommand = connection.CreateCommand(); + insertCommand.CommandText = """ + INSERT INTO "_workspace_meta" ("table_name", "document_id", "worksheet_name", "file_name", "source_names_json") + VALUES ($tableName, $documentId, NULL, 'data.csv', '{}') + """; + insertCommand.Parameters.AddWithValue("$tableName", tableName); + insertCommand.Parameters.AddWithValue("$documentId", documentId); + insertCommand.ExecuteNonQuery(); + } + + private static void LeaveDatabaseReadOnlyInThePool(string databasePath) + { + // Uses the pooled connection string on purpose: disposing this connection returns it to the + // pool still carrying query_only = ON, which is what production hit. + using var connection = new SqliteConnection($"Data Source={databasePath}"); + connection.Open(); + + using var command = connection.CreateCommand(); + command.CommandText = "PRAGMA query_only = ON"; + command.ExecuteNonQuery(); + } + + private static List GetUserTableNames(string databasePath) + { + var names = new List(); + + using var connection = OpenUnpooled(databasePath); + + using var command = connection.CreateCommand(); + command.CommandText = "SELECT name FROM sqlite_master WHERE type = 'table' AND name <> '_workspace_meta' ORDER BY name"; + + using var reader = command.ExecuteReader(); + + while (reader.Read()) + { + names.Add(reader.GetString(0)); + } + + return names; + } + + private static SqliteConnection OpenUnpooled(string databasePath) + { + var connection = new SqliteConnection($"Data Source={databasePath};Pooling=False"); + connection.Open(); + + return connection; + } + + private static void TryDeleteDirectory(string path) + { + try + { + Directory.Delete(path, recursive: true); + } + catch (IOException) + { + } + } + + private static TabularWorkspaceDocumentEventHandler CreateHandler( + Mock artifactStore, + string basePath = null) { var options = new ChatDocumentsOptions(); options.Add(new ExtractorExtension(".csv", embeddable: false, isTabular: true)); var fileStoreOptions = new DocumentFileSystemFileStoreOptions { - BasePath = Path.Combine(Path.GetTempPath(), "tabular-doc-event-tests"), + BasePath = basePath ?? Path.Combine(Path.GetTempPath(), "tabular-doc-event-tests"), }; return new TabularWorkspaceDocumentEventHandler( From 8a502e363fb1c81879fc50575115a3ff8c987075 Mon Sep 17 00:00:00 2001 From: Mike Alhayek Date: Mon, 14 Sep 2026 10:23:21 -0700 Subject: [PATCH 2/3] Confine a tabular workspace scope to a single directory Every chat session and chat interaction gets its own tabular.db, so isolation between conversations rests entirely on the reference type and reference id that form the path. Neither was validated where the path was built, and a reference id that walked out of its own directory resolved into a sibling scope: removing a document under "session-1/../session-2" dropped session-2's tables and deleted its database. DefaultConversationDocumentCleanupService already guarded its own copy of the path with a single-segment check. That check now lives in TabularWorkspaceDatabase alongside the path builders, so all four call sites are covered by one rule and the cleanup service no longer keeps a private duplicate of the layout. An unsafe scope resolves to no path at all, which leaves the workspace in-memory and makes the delete and drop handlers no-ops, rather than touching a database the scope does not own. Co-Authored-By: Claude Opus 5 --- ...faultConversationDocumentCleanupService.cs | 32 +++--------- .../Tabular/TabularWorkspaceDatabase.cs | 50 +++++++++++++++++-- ...bularWorkspaceDocumentEventHandlerTests.cs | 37 ++++++++++++++ 3 files changed, 90 insertions(+), 29 deletions(-) diff --git a/src/Primitives/CrestApps.Core.AI.Documents/Services/DefaultConversationDocumentCleanupService.cs b/src/Primitives/CrestApps.Core.AI.Documents/Services/DefaultConversationDocumentCleanupService.cs index 164fabdb..f1f16dec 100644 --- a/src/Primitives/CrestApps.Core.AI.Documents/Services/DefaultConversationDocumentCleanupService.cs +++ b/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; /// 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; @@ -142,15 +139,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 +166,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); - } } diff --git a/src/Primitives/CrestApps.Core.AI.Documents/Tabular/TabularWorkspaceDatabase.cs b/src/Primitives/CrestApps.Core.AI.Documents/Tabular/TabularWorkspaceDatabase.cs index 3dd54e56..71ef5582 100644 --- a/src/Primitives/CrestApps.Core.AI.Documents/Tabular/TabularWorkspaceDatabase.cs +++ b/src/Primitives/CrestApps.Core.AI.Documents/Tabular/TabularWorkspaceDatabase.cs @@ -1,3 +1,4 @@ +using System.Text.RegularExpressions; using Microsoft.Data.Sqlite; namespace CrestApps.Core.AI.Documents.Tabular; @@ -6,7 +7,7 @@ namespace CrestApps.Core.AI.Documents.Tabular; /// Centralizes how the file-backed tabular workspace database is located and opened so every caller /// agrees on the path layout, the connection string, and the connection's initial write state. /// -internal static class TabularWorkspaceDatabase +internal static partial class TabularWorkspaceDatabase { private const string DatabaseFileName = "tabular.db"; private const string DocumentsFolderName = "documents"; @@ -18,10 +19,10 @@ internal static class TabularWorkspaceDatabase /// The document file store base path. /// The reference type owning the workspace. /// The reference identifier owning the workspace. - /// The database path, or when the scope is incomplete. + /// The database path, or when the scope is incomplete or unsafe. public static string GetDatabasePath(string basePath, string referenceType, string referenceId) { - if (string.IsNullOrEmpty(basePath) || string.IsNullOrEmpty(referenceType) || string.IsNullOrEmpty(referenceId)) + if (string.IsNullOrEmpty(basePath) || !IsSafeScope(referenceType, referenceId)) { return null; } @@ -29,6 +30,49 @@ public static string GetDatabasePath(string basePath, string referenceType, stri return Path.Combine(basePath, DocumentsFolderName, referenceType, referenceId, DataFolderName, DatabaseFileName); } + /// + /// Builds the workspace database path relative to the document file store, using forward slashes. + /// + /// The reference type owning the workspace. + /// The reference identifier owning the workspace. + /// The store-relative path, or when the scope is incomplete or unsafe. + public static string GetStorageRelativePath(string referenceType, string referenceId) + { + if (!IsSafeScope(referenceType, referenceId)) + { + return null; + } + + return string.Join('/', DocumentsFolderName, referenceType, referenceId, DataFolderName, DatabaseFileName); + } + + /// + /// Determines whether a scope maps to exactly one directory of its own. + /// + /// + /// Each conversation scope owns a separate database file, so isolation between chat sessions and + /// chat interactions rests entirely on these two segments. Restricting them to a single path + /// segment keeps a scope from ever resolving into another scope's directory, whatever the + /// identifiers turn out to contain. + /// + /// The reference type owning the workspace. + /// The reference identifier owning the workspace. + /// when both segments are safe; otherwise . + private static bool IsSafeScope(string referenceType, string referenceId) + { + return IsSafePathSegment(referenceType) && IsSafePathSegment(referenceId); + } + + private static bool IsSafePathSegment(string value) + { + return !string.IsNullOrWhiteSpace(value) + && value is not "." and not ".." + && SafePathSegmentExpression().IsMatch(value); + } + + [GeneratedRegex("^[a-zA-Z0-9._-]+$")] + private static partial Regex SafePathSegmentExpression(); + /// /// Returns the database file and its write-ahead-log sidecars, which must be removed together. /// diff --git a/tests/CrestApps.Core.Tests/Core/Documents/Tabular/TabularWorkspaceDocumentEventHandlerTests.cs b/tests/CrestApps.Core.Tests/Core/Documents/Tabular/TabularWorkspaceDocumentEventHandlerTests.cs index 2c4ad9d5..f8992332 100644 --- a/tests/CrestApps.Core.Tests/Core/Documents/Tabular/TabularWorkspaceDocumentEventHandlerTests.cs +++ b/tests/CrestApps.Core.Tests/Core/Documents/Tabular/TabularWorkspaceDocumentEventHandlerTests.cs @@ -118,6 +118,43 @@ await handler.RemovedAsync(new AIChatDocumentRemoveContext } } + [Theory] + [InlineData("..")] + [InlineData("../session-2")] + [InlineData("session-1/../session-2")] + public async Task RemovedAsync_WhenTheScopeIsNotASinglePathSegment_LeavesEveryDatabaseAlone(string referenceId) + { + var basePath = CreateTemporaryBasePath(); + + try + { + // Each conversation scope owns its own database file, so a reference id that walks out of + // its directory would let one scope drop another scope's tables. + var databasePath = CreateWorkspaceDatabase(basePath, "session-2", "doc-1", "sales"); + + var artifactStore = new Mock(); + var handler = CreateHandler(artifactStore, basePath); + + await handler.RemovedAsync(new AIChatDocumentRemoveContext + { + ReferenceId = referenceId, + ReferenceType = AIReferenceTypes.Document.ChatSession, + DocumentInfo = new ChatDocumentInfo + { + DocumentId = "doc-1", + FileName = "data.csv", + }, + }, TestContext.Current.CancellationToken); + + Assert.True(File.Exists(databasePath)); + Assert.Equal(["sales"], GetUserTableNames(databasePath)); + } + finally + { + TryDeleteDirectory(basePath); + } + } + [Fact] public async Task RemovedAsync_WhenAnEarlierConnectionLeftTheDatabaseReadOnly_StillDropsTheDocumentTables() { From cebb30444a8f5736cdf3f9c15999c67aeb7e902c Mon Sep 17 00:00:00 2001 From: Mike Alhayek Date: Mon, 14 Sep 2026 10:40:03 -0700 Subject: [PATCH 3/3] Drop tabular tables once their document or conversation is gone MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A workspace table could outlive the document it came from. Nothing pruned the database: SynchronizeTablesAsync only ever added tables, and LoadMetadataFromDatabase exposed every row in _workspace_meta to the model. The removal handler was the only thing that dropped anything, so whenever it did not run, or failed the way the pooled read-only connection made it fail, the removed spreadsheet stayed queryable for the rest of the conversation. EnsureReadyAsync now drops every table whose document is no longer attached before it imports new ones. The caller passes the complete document set for the scope, so anything else in the database is detached by definition. This makes the workspace self-correcting rather than dependent on the removal handler having succeeded earlier. Deleting a conversation had a matching gap: CleanupAsync returned as soon as the document store reported no documents, which skipped the database delete entirely. A conversation whose spreadsheets were each removed individually therefore orphaned its tabular.db forever. The database is separate storage that outlives the document rows, so it is now deleted whether or not any documents remain. Tests cover all three delete paths and the isolation between them: tables pruned on the next request (including every table of a multi-worksheet document), reattaching a dropped document, conversation cleanup with and without documents, history clearing, and in each case that the neighbouring scope's database is untouched — a sibling conversation, and the same identifier under the other reference type. Co-Authored-By: Claude Opus 5 --- ...faultConversationDocumentCleanupService.cs | 10 +- .../Tabular/TabularWorkspace.cs | 75 +++++ ...ularWorkspaceHistoryClearedHandlerTests.cs | 124 ++++++++ ...ConversationDocumentCleanupServiceTests.cs | 99 +++++- .../Tabular/TabularWorkspaceLifecycleTests.cs | 286 ++++++++++++++++++ 5 files changed, 574 insertions(+), 20 deletions(-) create mode 100644 tests/CrestApps.Core.Tests/Core/Documents/Handlers/TabularWorkspaceHistoryClearedHandlerTests.cs create mode 100644 tests/CrestApps.Core.Tests/Core/Documents/Tabular/TabularWorkspaceLifecycleTests.cs diff --git a/src/Primitives/CrestApps.Core.AI.Documents/Services/DefaultConversationDocumentCleanupService.cs b/src/Primitives/CrestApps.Core.AI.Documents/Services/DefaultConversationDocumentCleanupService.cs index f1f16dec..1a657a1a 100644 --- a/src/Primitives/CrestApps.Core.AI.Documents/Services/DefaultConversationDocumentCleanupService.cs +++ b/src/Primitives/CrestApps.Core.AI.Documents/Services/DefaultConversationDocumentCleanupService.cs @@ -61,11 +61,6 @@ 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(); @@ -73,9 +68,12 @@ public async Task CleanupAsync(string referenceId, string referenceType, Cancell 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}'.", diff --git a/src/Primitives/CrestApps.Core.AI.Documents/Tabular/TabularWorkspace.cs b/src/Primitives/CrestApps.Core.AI.Documents/Tabular/TabularWorkspace.cs index 5c4a7c33..fc3e479b 100644 --- a/src/Primitives/CrestApps.Core.AI.Documents/Tabular/TabularWorkspace.cs +++ b/src/Primitives/CrestApps.Core.AI.Documents/Tabular/TabularWorkspace.cs @@ -120,6 +120,7 @@ public async Task> EnsureReadyAsync( try { + RemoveTablesForDetachedDocuments(documents); await SynchronizeTablesAsync(documents, artifactLoader, workspaceImporter, cancellationToken); } finally @@ -646,6 +647,80 @@ private async Task SynchronizeTablesAsync( } } + /// + /// Drops every table whose document is no longer attached to the conversation. + /// + /// + /// 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. + /// + /// The documents currently attached to the conversation. + private void RemoveTablesForDetachedDocuments(IReadOnlyList documents) + { + if (_tables.Count == 0) + { + return; + } + + var attached = new HashSet(StringComparer.Ordinal); + + foreach (var document in documents) + { + attached.Add(document.DocumentId); + } + + List 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) diff --git a/tests/CrestApps.Core.Tests/Core/Documents/Handlers/TabularWorkspaceHistoryClearedHandlerTests.cs b/tests/CrestApps.Core.Tests/Core/Documents/Handlers/TabularWorkspaceHistoryClearedHandlerTests.cs new file mode 100644 index 00000000..3e0cd701 --- /dev/null +++ b/tests/CrestApps.Core.Tests/Core/Documents/Handlers/TabularWorkspaceHistoryClearedHandlerTests.cs @@ -0,0 +1,124 @@ +using CrestApps.Core.AI.Documents; +using CrestApps.Core.AI.Documents.Handlers; +using CrestApps.Core.AI.Models; +using Microsoft.Extensions.Logging.Abstractions; +using Microsoft.Extensions.Options; + +namespace CrestApps.Core.Tests.Core.Documents.Handlers; + +/// +/// Clearing a chat interaction's history must take its workspace database with it, so the next tool +/// call rebuilds from the original uploaded files instead of serving mutations from the cleared +/// conversation — and must never reach a database that belongs to a different interaction. +/// +public sealed class TabularWorkspaceHistoryClearedHandlerTests : IDisposable +{ + private readonly string _basePath; + + public TabularWorkspaceHistoryClearedHandlerTests() + { + _basePath = Path.Combine(Path.GetTempPath(), "tabular-history-cleared-tests", Path.GetRandomFileName()); + Directory.CreateDirectory(_basePath); + } + + public void Dispose() + { + try + { + Directory.Delete(_basePath, recursive: true); + } + catch (IOException) + { + } + } + + [Fact] + public async Task HistoryClearedAsync_DeletesTheDatabaseAndItsWriteAheadLogSidecars() + { + var paths = CreateDatabaseFiles("interaction-1"); + + await CreateHandler().HistoryClearedAsync( + new ChatInteraction { ItemId = "interaction-1" }, + [], + TestContext.Current.CancellationToken); + + Assert.All(paths, path => Assert.False(File.Exists(path), $"Expected '{path}' to be deleted.")); + } + + [Fact] + public async Task HistoryClearedAsync_LeavesOtherInteractionsDatabasesAlone() + { + var cleared = CreateDatabaseFiles("interaction-1"); + var untouched = CreateDatabaseFiles("interaction-2"); + + await CreateHandler().HistoryClearedAsync( + new ChatInteraction { ItemId = "interaction-1" }, + [], + TestContext.Current.CancellationToken); + + Assert.All(cleared, path => Assert.False(File.Exists(path))); + Assert.All(untouched, path => Assert.True(File.Exists(path), $"Expected '{path}' to survive.")); + } + + [Fact] + public async Task HistoryClearedAsync_DoesNotReachTheChatSessionDatabaseOfTheSameIdentifier() + { + var interactionFiles = CreateDatabaseFiles("shared-id"); + var sessionFiles = CreateDatabaseFiles("shared-id", "chat-session"); + + await CreateHandler().HistoryClearedAsync( + new ChatInteraction { ItemId = "shared-id" }, + [], + TestContext.Current.CancellationToken); + + Assert.All(interactionFiles, path => Assert.False(File.Exists(path))); + Assert.All(sessionFiles, path => Assert.True(File.Exists(path), $"Expected '{path}' to survive.")); + } + + [Theory] + [InlineData("..")] + [InlineData("interaction-1/../interaction-2")] + public async Task HistoryClearedAsync_WhenTheInteractionIdIsNotASinglePathSegment_DeletesNothing(string itemId) + { + var files = CreateDatabaseFiles("interaction-2"); + + await CreateHandler().HistoryClearedAsync( + new ChatInteraction { ItemId = itemId }, + [], + TestContext.Current.CancellationToken); + + Assert.All(files, path => Assert.True(File.Exists(path), $"Expected '{path}' to survive.")); + } + + [Fact] + public async Task HistoryClearedAsync_WhenTheInteractionIsMissing_DoesNothing() + { + var files = CreateDatabaseFiles("interaction-1"); + + await CreateHandler().HistoryClearedAsync(null, [], TestContext.Current.CancellationToken); + + Assert.All(files, path => Assert.True(File.Exists(path))); + } + + private string[] CreateDatabaseFiles(string referenceId, string referenceType = "chat-interaction") + { + var databasePath = Path.Combine(_basePath, "documents", referenceType, referenceId, "data", "tabular.db"); + Directory.CreateDirectory(Path.GetDirectoryName(databasePath)); + + string[] paths = [databasePath, databasePath + "-wal", databasePath + "-shm"]; + + foreach (var path in paths) + { + File.WriteAllText(path, "content"); + } + + return paths; + } + + private TabularWorkspaceHistoryClearedHandler CreateHandler() + { + return new TabularWorkspaceHistoryClearedHandler( + Options.Create(new DocumentFileSystemFileStoreOptions { BasePath = _basePath }), + NullLogger.Instance); + } +} diff --git a/tests/CrestApps.Core.Tests/Core/Documents/Services/ConversationDocumentCleanupServiceTests.cs b/tests/CrestApps.Core.Tests/Core/Documents/Services/ConversationDocumentCleanupServiceTests.cs index f12c18a2..91b282e1 100644 --- a/tests/CrestApps.Core.Tests/Core/Documents/Services/ConversationDocumentCleanupServiceTests.cs +++ b/tests/CrestApps.Core.Tests/Core/Documents/Services/ConversationDocumentCleanupServiceTests.cs @@ -46,35 +46,106 @@ public async Task CleanupAsync_RemovesDocumentsFilesArtifactsAndChunks() fileStore.Verify(store => store.DeleteFileAsync(document.StoredFilePath), Times.Once); documentStore.Verify(store => store.DeleteAsync(document, It.IsAny()), Times.Once); } + + // The workspace database holds a copy of every spreadsheet, so deleting the conversation has + // to take the database with it. + foreach (var path in TabularDatabasePaths("chat-session", "session-1")) + { + fileStore.Verify(store => store.DeleteFileAsync(path), Times.Once); + } } [Fact] - public async Task CleanupAsync_WhenNoDocuments_DoesNothing() + public async Task CleanupAsync_WhenNoDocumentsRemain_StillDeletesTheTabularDatabase() { - var documentStore = new Mock(MockBehavior.Strict); - var chunkStore = new Mock(MockBehavior.Strict); - var fileStore = new Mock(MockBehavior.Strict); - var artifactStore = new Mock(MockBehavior.Strict); + var documentStore = new Mock(); + var fileStore = new Mock(); + // The conversation's spreadsheets were each removed individually, so no document rows remain. + // The database file is separate storage and survives that, so cleanup still has to delete it. documentStore .Setup(store => store.GetDocumentsAsync("session-1", "chat-session")) .ReturnsAsync([]); - var service = new DefaultConversationDocumentCleanupService( + var service = CreateService(documentStore, fileStore); + + await service.CleanupAsync("session-1", "chat-session", TestContext.Current.CancellationToken); + + foreach (var path in TabularDatabasePaths("chat-session", "session-1")) + { + fileStore.Verify(store => store.DeleteFileAsync(path), Times.Once); + } + } + + [Theory] + [InlineData("chat-session")] + [InlineData("chat-interaction")] + public async Task CleanupAsync_DeletesOnlyTheTabularDatabaseOfTheScopeBeingCleaned(string referenceType) + { + var documentStore = new Mock(); + var fileStore = new Mock(); + + documentStore + .Setup(store => store.GetDocumentsAsync(It.IsAny(), It.IsAny())) + .ReturnsAsync([]); + + var service = CreateService(documentStore, fileStore); + + await service.CleanupAsync("scope-1", referenceType, TestContext.Current.CancellationToken); + + foreach (var path in TabularDatabasePaths(referenceType, "scope-1")) + { + fileStore.Verify(store => store.DeleteFileAsync(path), Times.Once); + } + + // Nothing outside the cleaned scope may be touched: not a sibling conversation, and not the + // same identifier under the other reference type. + var otherType = referenceType == "chat-session" ? "chat-interaction" : "chat-session"; + + foreach (var path in TabularDatabasePaths(referenceType, "scope-2").Concat(TabularDatabasePaths(otherType, "scope-1"))) + { + fileStore.Verify(store => store.DeleteFileAsync(path), Times.Never); + } + } + + [Theory] + [InlineData("..")] + [InlineData("scope-1/../scope-2")] + [InlineData("scope 1")] + public async Task CleanupAsync_WhenTheScopeIsNotASinglePathSegment_DeletesNoDatabase(string referenceId) + { + var documentStore = new Mock(); + var fileStore = new Mock(); + + documentStore + .Setup(store => store.GetDocumentsAsync(It.IsAny(), It.IsAny())) + .ReturnsAsync([]); + + var service = CreateService(documentStore, fileStore); + + await service.CleanupAsync(referenceId, "chat-session", TestContext.Current.CancellationToken); + + fileStore.Verify(store => store.DeleteFileAsync(It.IsAny()), Times.Never); + } + + private static DefaultConversationDocumentCleanupService CreateService( + Mock documentStore, + Mock fileStore) + { + return new DefaultConversationDocumentCleanupService( documentStore.Object, - chunkStore.Object, + Mock.Of(), fileStore.Object, - artifactStore.Object, + Mock.Of(), Options.Create(new DocumentFileSystemFileStoreOptions { BasePath = Path.Combine(Path.GetTempPath(), "cleanup-tests") }), NullLogger.Instance); + } - await service.CleanupAsync("session-1", "chat-session", TestContext.Current.CancellationToken); + private static string[] TabularDatabasePaths(string referenceType, string referenceId) + { + var databasePath = $"documents/{referenceType}/{referenceId}/data/tabular.db"; - documentStore.Verify(store => store.GetDocumentsAsync("session-1", "chat-session"), Times.Once); - documentStore.VerifyNoOtherCalls(); - chunkStore.VerifyNoOtherCalls(); - fileStore.VerifyNoOtherCalls(); - artifactStore.VerifyNoOtherCalls(); + return [databasePath, databasePath + "-wal", databasePath + "-shm"]; } [Theory] diff --git a/tests/CrestApps.Core.Tests/Core/Documents/Tabular/TabularWorkspaceLifecycleTests.cs b/tests/CrestApps.Core.Tests/Core/Documents/Tabular/TabularWorkspaceLifecycleTests.cs new file mode 100644 index 00000000..bdaf7fb9 --- /dev/null +++ b/tests/CrestApps.Core.Tests/Core/Documents/Tabular/TabularWorkspaceLifecycleTests.cs @@ -0,0 +1,286 @@ +using CrestApps.Core.AI.Documents.Tabular; +using Microsoft.Data.Sqlite; + +namespace CrestApps.Core.Tests.Core.Documents.Tabular; + +/// +/// Covers what happens to a file-backed workspace database across requests: tables must not outlive +/// the document they came from, and one conversation's database must never be reachable from another. +/// +public sealed class TabularWorkspaceLifecycleTests : IDisposable +{ + private const string SalesCsv = "region,amount\nNorth,100\nSouth,200"; + private const string BudgetCsv = "team,total\nOps,10\nEng,20"; + + private readonly string _root; + + public TabularWorkspaceLifecycleTests() + { + _root = Path.Combine(Path.GetTempPath(), "tabular-lifecycle-tests", Path.GetRandomFileName()); + Directory.CreateDirectory(_root); + } + + public void Dispose() + { + try + { + Directory.Delete(_root, recursive: true); + } + catch (IOException) + { + } + } + + [Fact] + public async Task EnsureReadyAsync_WhenADocumentIsNoLongerAttached_DropsItsTablesOnTheNextRequest() + { + var cancellationToken = TestContext.Current.CancellationToken; + var databasePath = DatabasePath("session-1"); + + using (var workspace = CreateWorkspace(databasePath)) + { + await workspace.EnsureReadyAsync( + [Document("doc-1", "sales.csv"), Document("doc-2", "budget.csv")], + LoaderFor(("doc-1", SalesCsv), ("doc-2", BudgetCsv)), + cancellationToken); + } + + Assert.Equal(["budget", "sales"], GetUserTableNames(databasePath)); + + // The next request resolves only doc-1, so doc-2 is gone from the conversation. + using (var workspace = CreateWorkspace(databasePath)) + { + var tables = await workspace.EnsureReadyAsync( + [Document("doc-1", "sales.csv")], + LoaderFor(("doc-1", SalesCsv)), + cancellationToken); + + var table = Assert.Single(tables); + Assert.Equal("sales", table.TableName); + } + + // The table is really gone from the database, not merely hidden from the model. + Assert.Equal(["sales"], GetUserTableNames(databasePath)); + Assert.Equal(["sales"], GetMetadataTableNames(databasePath)); + } + + [Fact] + public async Task EnsureReadyAsync_WhenEveryDocumentIsDetached_DropsEveryTable() + { + var cancellationToken = TestContext.Current.CancellationToken; + var databasePath = DatabasePath("session-1"); + + using (var workspace = CreateWorkspace(databasePath)) + { + await workspace.EnsureReadyAsync( + [Document("doc-1", "sales.csv")], + LoaderFor(("doc-1", SalesCsv)), + cancellationToken); + } + + using (var workspace = CreateWorkspace(databasePath)) + { + var tables = await workspace.EnsureReadyAsync([], LoaderFor(), cancellationToken); + + Assert.Empty(tables); + } + + Assert.Empty(GetUserTableNames(databasePath)); + Assert.Empty(GetMetadataTableNames(databasePath)); + } + + [Fact] + public async Task EnsureReadyAsync_WhenADetachedDocumentHadSeveralWorksheets_DropsAllOfItsTables() + { + var cancellationToken = TestContext.Current.CancellationToken; + var databasePath = DatabasePath("session-1"); + + var multiSheet = new TabularDocumentArtifact + { + Worksheets = + [ + new TabularWorksheet { Name = "Q1", Header = ["region", "amount"], Rows = [["North", "100"]] }, + new TabularWorksheet { Name = "Q2", Header = ["region", "amount"], Rows = [["South", "200"]] }, + ], + }; + + using (var workspace = CreateWorkspace(databasePath)) + { + await workspace.EnsureReadyAsync( + [Document("doc-1", "sales.xlsx"), Document("doc-2", "budget.csv")], + (document, _) => Task.FromResult( + document.DocumentId == "doc-1" + ? multiSheet + : TabularDocumentArtifact.FromDelimitedContent(BudgetCsv, document.FileName)), + null, + cancellationToken); + } + + Assert.Equal(["budget", "sales_Q1", "sales_Q2"], GetUserTableNames(databasePath)); + + using (var workspace = CreateWorkspace(databasePath)) + { + await workspace.EnsureReadyAsync( + [Document("doc-2", "budget.csv")], + LoaderFor(("doc-2", BudgetCsv)), + cancellationToken); + } + + Assert.Equal(["budget"], GetUserTableNames(databasePath)); + } + + [Fact] + public async Task EnsureReadyAsync_DoesNotReachAnotherConversationsDatabase() + { + var cancellationToken = TestContext.Current.CancellationToken; + var first = DatabasePath("session-1"); + var second = DatabasePath("session-2"); + + using (var workspace = CreateWorkspace(first)) + { + await workspace.EnsureReadyAsync( + [Document("doc-1", "sales.csv")], + LoaderFor(("doc-1", SalesCsv)), + cancellationToken); + } + + using (var workspace = CreateWorkspace(second)) + { + await workspace.EnsureReadyAsync( + [Document("doc-2", "budget.csv")], + LoaderFor(("doc-2", BudgetCsv)), + cancellationToken); + } + + // Two conversations, two files, neither aware of the other's tables. + Assert.Equal(["sales"], GetUserTableNames(first)); + Assert.Equal(["budget"], GetUserTableNames(second)); + + // Detaching everything from the first conversation leaves the second untouched. + using (var workspace = CreateWorkspace(first)) + { + await workspace.EnsureReadyAsync([], LoaderFor(), cancellationToken); + } + + Assert.Empty(GetUserTableNames(first)); + Assert.Equal(["budget"], GetUserTableNames(second)); + } + + [Fact] + public async Task EnsureReadyAsync_ASecondConversationNeverSeesTheFirstConversationsDocument() + { + var cancellationToken = TestContext.Current.CancellationToken; + var first = DatabasePath("session-1"); + var second = DatabasePath("session-2"); + + using (var workspace = CreateWorkspace(first)) + { + await workspace.EnsureReadyAsync( + [Document("doc-1", "sales.csv")], + LoaderFor(("doc-1", SalesCsv)), + cancellationToken); + } + + using var other = CreateWorkspace(second); + var tables = await other.EnsureReadyAsync( + [Document("doc-2", "budget.csv")], + LoaderFor(("doc-2", BudgetCsv)), + cancellationToken); + + var table = Assert.Single(tables); + Assert.Equal("budget", table.TableName); + + // The other conversation's table is not queryable from here. + var exception = await Assert.ThrowsAsync( + () => other.QueryAsync("SELECT * FROM sales", 10, cancellationToken)); + + Assert.Contains("no such table", exception.Message, StringComparison.OrdinalIgnoreCase); + } + + [Fact] + public async Task EnsureReadyAsync_ReattachingADroppedDocument_RebuildsItsTable() + { + var cancellationToken = TestContext.Current.CancellationToken; + var databasePath = DatabasePath("session-1"); + + using (var workspace = CreateWorkspace(databasePath)) + { + await workspace.EnsureReadyAsync( + [Document("doc-1", "sales.csv")], + LoaderFor(("doc-1", SalesCsv)), + cancellationToken); + } + + using (var workspace = CreateWorkspace(databasePath)) + { + await workspace.EnsureReadyAsync([], LoaderFor(), cancellationToken); + } + + using (var workspace = CreateWorkspace(databasePath)) + { + var tables = await workspace.EnsureReadyAsync( + [Document("doc-1", "sales.csv")], + LoaderFor(("doc-1", SalesCsv)), + cancellationToken); + + var table = Assert.Single(tables); + Assert.Equal("sales", table.TableName); + Assert.Equal(2, table.RowCount); + } + } + + private string DatabasePath(string referenceId) + { + var path = Path.Combine(_root, "documents", "chat-session", referenceId, "data", "tabular.db"); + Directory.CreateDirectory(Path.GetDirectoryName(path)); + + return path; + } + + private static TabularWorkspace CreateWorkspace(string databasePath) + { + return new TabularWorkspace(new TabularWorkspaceOptions(), databasePath); + } + + private static TabularDocumentRef Document(string documentId, string fileName) + { + return new TabularDocumentRef(documentId, fileName); + } + + private static Func> LoaderFor(params (string DocumentId, string Content)[] contents) + { + var map = contents.ToDictionary(c => c.DocumentId, c => c.Content, StringComparer.Ordinal); + + return (documentId, _) => Task.FromResult(map.TryGetValue(documentId, out var content) ? content : string.Empty); + } + + private static List GetUserTableNames(string databasePath) + { + return ReadStrings(databasePath, "SELECT name FROM sqlite_master WHERE type = 'table' AND name <> '_workspace_meta' ORDER BY name"); + } + + private static List GetMetadataTableNames(string databasePath) + { + return ReadStrings(databasePath, "SELECT table_name FROM \"_workspace_meta\" ORDER BY table_name"); + } + + private static List ReadStrings(string databasePath, string sql) + { + var values = new List(); + + using var connection = new SqliteConnection($"Data Source={databasePath};Pooling=False"); + connection.Open(); + + using var command = connection.CreateCommand(); + command.CommandText = sql; + + using var reader = command.ExecuteReader(); + + while (reader.Read()) + { + values.Add(reader.GetString(0)); + } + + return values; + } +}