All critical bugs (thread‑safety, transaction scope, tag filtering, …

.github/workflows/dotnet.yml

@@ -54,10 +54,6 @@ jobs:
 
     if: startsWith(github.ref, 'refs/tags/v')
 
-    permissions:
-      id-token: write
-      contents: read
-
     steps:
       - uses: actions/checkout@v4
 
@@ -81,24 +77,20 @@ jobs:
       - name: List nupkg files
         run: ls -la ./nupkgs
 
-      - name: Get OIDC token
-        id: oidc
-        uses: actions/github-script@v7
-        with:
-          script: |
-            const token = await core.getIDToken('api://NuGet');
-            core.setOutput('token', token);
-
       - name: Push to NuGet.org
         env:
-          NUGET_AUTH_TOKEN: ${{ steps.oidc.outputs.token }}
+          NUGET_API_KEY: ${{ secrets.NUGET_API_KEY }}
         run: |
+          if [ -z "$NUGET_API_KEY" ]; then
+            echo "Error: NUGET_API_KEY secret is not set."
+            exit 1
+          fi
           for pkg in ./nupkgs/*.nupkg; do
             if [ -f "$pkg" ]; then
               echo "Pushing $pkg"
               dotnet nuget push "$pkg" \
                 --source "https://api.nuget.org/v3/index.json" \
-                --api-key "$NUGET_AUTH_TOKEN" \
+                --api-key "$NUGET_API_KEY" \
                 --skip-duplicate
             else
               echo "No .nupkg files found"

CHANGELOG.md

@@ -5,6 +5,32 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+---
+
+## [1.0.1] — 2026-07-12
+
+Patch release fixing all critical correctness bugs identified in the initial release. No public API surface changes.
+
+### Fixed
+
+- **`SqlServerStoreBase` thread-safety** — the store previously held a single shared `SqlConnection` instance, which is not thread-safe. Concurrent calls to any SQL Server store method would corrupt each other's state. The owned-connection path now creates a fresh `SqlConnection` per call (matching the connection pool pattern used by the Postgres store); externally supplied connections are guarded with a `SemaphoreSlim(1,1)`.
+
+- **`PostgresModelStore.LoadAsync` transaction scope** — large object reads were happening outside the transaction that produced the OIDs. After `reader.CloseAsync()`, the metadata transaction was closed, leaving the OIDs as dangling references that could be unlinked by a concurrent writer before the read completed. The metadata read and both `ReadLargeObjectAsync` calls are now wrapped in a single transaction, passed explicitly through the call chain.
+
+- **`FileSystemModelStore.ListAsync` and `FileSystemSessionStore.ListAsync` silently ignored tag filters** — calling `ListAsync(tagKey: "env", tagValue: "prod")` returned all checkpoints regardless of tags, with no error or warning. Both stores now load each `manifest.json` / `meta.json` in memory and filter by the requested tag key/value pair before returning results.
+
+- **`BackgroundSaver.Enqueue` blocked the caller synchronously** — the previous implementation called `.GetAwaiter().GetResult()` on the channel `WriteAsync`, which blocked the calling thread whenever the bounded queue was full. This defeated the purpose of background saves and introduced deadlock risk on the thread pool. `Enqueue` has been replaced with `EnqueueAsync` returning `ValueTask`; both `CheckpointManager` and `SessionManager` now `await` it correctly.
+
+- **Postgres connections were never returned to the pool** — `GetConnectionAsync` opened a connection from the `NpgsqlDataSource` pool but no call site disposed it, preventing connections from being returned. All call sites across `PostgresModelStore` and `PostgresSessionStore` now use `using var connection = await GetConnectionAsync(...)` to ensure prompt return to the pool.
+
+- **SQL Server tag filtering produced incorrect results** — tag queries used `LIKE '%"key":"value"%'` string matching against the JSON column, which failed when the serializer emitted spaces and produced false positives when a value contained the search string as a substring. Replaced with `JSON_VALUE(Tags, '$.{key}') = @TagValue` for correct, indexed JSON field extraction (requires SQL Server 2016+).
+
+- **`TokenizerData.MergeRules` did not serialise correctly** — the field was typed as `List<(string Left, string Right)>?` (C# value tuples), which `System.Text.Json` does not serialise by default, producing `{"Item1":"...","Item2":"..."}` or failing silently depending on runtime version. Changed to `List<MergeRule>?` using the existing `MergeRule` class, which serialises correctly in all versions.
+
+- **SQL Server stores were in the wrong namespace** — `SqlServerStoreBase`, `SqlServerModelStore`, and `SqlServerSessionStore` were in `StateCheckpoint.NET.Stores.Mysql`. Corrected to `StateCheckpoint.NET.Stores` with files moved to `Stores/SqlServer/`.
+
+- PostgresModelStore and PostgresSessionStore connections not disposed asynchronously — four call sites in PostgresModelStore and five in PostgresSessionStore used using var connection (synchronous IDisposable) instead of await using var connection (IAsyncDisposable). NpgsqlConnection implements IAsyncDisposable and disposing it synchronously blocks the thread pool while the connection is returned. All call sites now use await using.
+
 ## [1.0.0] - 2025-06-25
 
 ### Added

StateCheckpoint.NET/StateCheckpoint.NET.Tests/Background/BackgroundSaverTests.cs

@@ -13,7 +13,7 @@ public async Task Constructor_ShouldStartWorkerTask()
 
         // Assert
         var tcs = new TaskCompletionSource<bool>();
-        saver.Enqueue(async (ct) =>
+        await saver.EnqueueAsync(async (ct) =>
         {
             await Task.Delay(10, ct);
             tcs.SetResult(true);
@@ -32,7 +32,7 @@ public async Task Enqueue_ShouldAddOperationToQueue()
         var invoked = false;
 
         // Act
-        saver.Enqueue(async (ct) =>
+        await saver.EnqueueAsync(async (ct) =>
         {
             await Task.CompletedTask;
             invoked = true;
@@ -53,10 +53,10 @@ public async Task Enqueue_WhenDisposed_ShouldThrowInvalidOperationException()
         await saver.DisposeAsync();
 
         // Act
-        Action act = () => saver.Enqueue(async (ct) => await Task.CompletedTask);
+        var act = async () => await saver.EnqueueAsync(async (ct) => await Task.CompletedTask);
 
         // Assert
-        act.Should().Throw<ObjectDisposedException>();
+        await act.Should().ThrowAsync<InvalidOperationException>();
     }
 
     [Fact]
@@ -67,7 +67,7 @@ public async Task Enqueue_WhenCapacityReached_ShouldBlockUntilSpaceFrees()
         using var holdEvent = new ManualResetEventSlim(false);
 
         // 1st enqueue – worker picks it up and blocks synchronously.
-        saver.Enqueue(async (ct) =>
+        await saver.EnqueueAsync(async (ct) =>
         {
             holdEvent.Wait(ct);  // Blocks the worker thread until we signal.
             await Task.CompletedTask;
@@ -77,10 +77,10 @@ public async Task Enqueue_WhenCapacityReached_ShouldBlockUntilSpaceFrees()
         await Task.Delay(100);
 
         // 2nd enqueue – worker is blocked, so this fills the channel (capacity = 1).
-        saver.Enqueue(async (ct) => await Task.CompletedTask);
+        await saver.EnqueueAsync(async (ct) => await Task.CompletedTask);
 
         // 3rd enqueue – channel is full, so this MUST block.
-        var enqueueTask = Task.Run(() => saver.Enqueue(async (ct) => await Task.CompletedTask));
+        var enqueueTask = Task.Run(async () => await saver.EnqueueAsync(async (ct) => await Task.CompletedTask));
 
         // Verify the third enqueue is blocked.
         await Task.Delay(100);
@@ -104,7 +104,7 @@ public async Task ErrorHandling_ShouldInvokeOnErrorCallback()
 
         // Act
         var expectedException = new InvalidOperationException("Test error");
-        saver.Enqueue(async (ct) =>
+        await saver.EnqueueAsync(async (ct) =>
         {
             await Task.CompletedTask;
             throw expectedException;
@@ -127,7 +127,7 @@ public async Task ErrorHandling_WhenNoCallbackProvided_ShouldSwallowException()
         // Act & Assert - should not throw
         var act = async () =>
         {
-            saver.Enqueue(async (ct) =>
+            await saver.EnqueueAsync(async (ct) =>
             {
                 await Task.CompletedTask;
                 throw new InvalidOperationException("Test error");
@@ -146,7 +146,7 @@ public async Task DisposeAsync_ShouldCancelWorkerAndWaitForCompletion()
         var saver = new BackgroundSaver<object>(capacity: 2);
         var processed = false;
 
-        saver.Enqueue(async (ct) =>
+        await saver.EnqueueAsync(async (ct) =>
         {
             await Task.Delay(200, ct);
             processed = true;
@@ -166,7 +166,7 @@ public async Task DisposeAsync_ShouldCompleteAllPendingTasks()
         var saver = new BackgroundSaver<object>(capacity: 2);
         var tcs = new TaskCompletionSource<bool>();
 
-        saver.Enqueue(async (ct) =>
+        await saver.EnqueueAsync(async (ct) =>
         {
             await Task.Delay(50, ct);
             tcs.SetResult(true);
@@ -188,17 +188,17 @@ public async Task DisposeAsync_ShouldCancelAfterEnqueue()
         var saver = new BackgroundSaver<object>(capacity: 1);
 
         // Act
-        saver.Enqueue(async (ct) =>
+        await saver.EnqueueAsync(async (ct) =>
         {
             await Task.Delay(1000, ct);
         });
 
         await saver.DisposeAsync();
 
         // Assert
-        Action act = () => saver.Enqueue(async (ct) => await Task.CompletedTask);
+        Func<Task> act = async () => await saver.EnqueueAsync(async (ct) => await Task.CompletedTask);
 
-        act.Should().Throw<ObjectDisposedException>();
+        await act.Should().ThrowAsync<ObjectDisposedException>();
     }
 }
 

StateCheckpoint.NET/StateCheckpoint.NET.Tests/Integration/PostgresTests.cs

@@ -1,5 +1,5 @@
 using StateCheckpoint.NET.Manager;
-using StateCheckpoint.NET.Stores.Postgres;
+using StateCheckpoint.NET.Stores;
 using Npgsql;
 using Testcontainers.PostgreSql;
 

StateCheckpoint.NET/StateCheckpoint.NET.Tests/Integration/SqlServerTests.cs

@@ -1,6 +1,5 @@
 using StateCheckpoint.NET.Manager;
 using StateCheckpoint.NET.Stores;
-using StateCheckpoint.NET.Stores.Mysql;
 using Microsoft.Data.SqlClient;
 using Testcontainers.MsSql;
 

StateCheckpoint.NET/StateCheckpoint.NET.Tests/Stores/FileSystem/FileSystemModelStoreTests.cs

@@ -1,6 +1,6 @@
 using StateCheckpoint.NET.Models;
 using StateCheckpoint.NET.Settings;
-using StateCheckpoint.NET.Stores.FileSystem;
+using StateCheckpoint.NET.Stores;
 using FluentAssertions;
 
 namespace StateCheckpoint.NET.Tests.Stores.FileSystem;
@@ -280,18 +280,22 @@ public async Task ListAsync_WhenNoCheckpoints_ShouldReturnEmpty()
     }
 
     [Fact]
-    public async Task ListAsync_WithTagFilter_ShouldIgnoreTagsAndReturnAll()
+    public async Task ListAsync_WithTagFilter_ShouldFilterByTag()
     {
         // Arrange
         var store = new FileSystemModelStore(_testRoot, _defaultOptions);
         var id1 = Guid.NewGuid();
-        await store.SaveAsync(new ModelCheckpoint { ModelId = id1, Tags = new Dictionary<string, string> { { "key", "value" } } });
+        var id2 = Guid.NewGuid();
+
+        await store.SaveAsync(new ModelCheckpoint { ModelId = id1, Tags = new Dictionary<string, string> { { "env", "prod" } } });
+        await store.SaveAsync(new ModelCheckpoint { ModelId = id2, Tags = new Dictionary<string, string> { { "env", "dev" } } });
 
         // Act
-        var ids = await store.ListAsync("key", "value");
+        var ids = await store.ListAsync("env", "prod");
 
-        // Assert - FileSystem store ignores tag filtering, so it returns all.
-        ids.Should().Contain(id1);
+        // Assert
         ids.Count.Should().Be(1);
+        ids.Should().Contain(id1);
+        ids.Should().NotContain(id2);
     }
 }

StateCheckpoint.NET/StateCheckpoint.NET.Tests/Stores/FileSystem/FileSystemSessionStoreTests.cs

@@ -1,6 +1,6 @@
 using StateCheckpoint.NET.Models;
 using StateCheckpoint.NET.Settings;
-using StateCheckpoint.NET.Stores.FileSystem;
+using StateCheckpoint.NET.Stores;
 using FluentAssertions;
 
 namespace StateCheckpoint.NET.Tests.Stores.FileSystem;
@@ -30,7 +30,7 @@ public void Dispose()
     }
 
     // Helper to create a test session
-    private SessionCheckpoint CreateTestSession(Guid? id = null)
+    private SessionCheckpoint CreateTestSession(Guid? id = null, Dictionary<string, string>? tags = null)
     {
         return new SessionCheckpoint
         {
@@ -40,7 +40,7 @@ private SessionCheckpoint CreateTestSession(Guid? id = null)
             ModelFingerprint = "test-model-v1",
             SamplingConfig = new SamplingData { Temperature = 0.9f, TopP = 0.85f },
             LastUpdated = DateTime.UtcNow,
-            Tags = new Dictionary<string, string> { { "env", "test" } }
+            Tags = tags ?? new Dictionary<string, string> { { "env", "test" } }
         };
     }
 
@@ -237,18 +237,22 @@ public async Task ListAsync_WhenNoSessions_ShouldReturnEmpty()
     }
 
     [Fact]
-    public async Task ListAsync_WithTagFilter_ShouldIgnoreTagsAndReturnAll()
+    public async Task ListAsync_WithTagFilter_ShouldFilterByTag()
     {
         // Arrange
         var store = new FileSystemSessionStore(_testRoot, _defaultOptions);
-        var session = CreateTestSession();
-        await store.SaveAsync(session);
+        var session1 = CreateTestSession(tags: new Dictionary<string, string> { { "env", "prod" } });
+        var session2 = CreateTestSession(tags: new Dictionary<string, string> { { "env", "dev" } });
+
+        await store.SaveAsync(session1);
+        await store.SaveAsync(session2);
 
         // Act
-        var ids = await store.ListAsync("some", "filter");
+        var ids = await store.ListAsync("env", "prod");
 
-        // Assert - FileSystem store ignores tag filtering, so it returns all.
-        ids.Should().Contain(session.SessionId);
+        // Assert
         ids.Count.Should().Be(1);
+        ids.Should().Contain(session1.SessionId);
+        ids.Should().NotContain(session2.SessionId);
     }
 }