Server: collect tombstones past their retention, compact superseded change entries, and refuse cursors from before a collection

src/DodoSSH.Api/Features/Sync/SyncLog.cs

@@ -27,4 +27,16 @@ internal static partial class SyncLog
         int appliedCount,
         int operationCount,
         Guid vaultId);
+
+    [LoggerMessage(
+        EventId = 2003,
+        Level = LogLevel.Information,
+        Message = "Collected {Count} expired tombstone(s) and superseded change-log entries.")]
+    internal static partial void Collected(ILogger logger, int count);
+
+    [LoggerMessage(
+        EventId = 2004,
+        Level = LogLevel.Warning,
+        Message = "Tombstone collection failed; it will run again at the next interval.")]
+    internal static partial void CollectionFailed(ILogger logger, Exception exception);
 }

src/DodoSSH.Api/Features/Sync/SyncService.cs

@@ -62,6 +62,14 @@ internal sealed class SyncService(
             changes.RemoveAt(changes.Count - 1);
         }
 
+        // ◆ After the read, not before it. The collector deletes a tombstone's entries and raises the horizon
+        // in one transaction, so a read that already missed an entry is followed by a horizon that says so;
+        // checked first, the two could straddle a collection and the deletion would go unnoticed for good.
+        if (afterSequence > 0 && afterSequence < await HorizonAsync(vault.Id, cancellationToken).ConfigureAwait(false))
+        {
+            throw new InvalidCursorException();
+        }
+
         var hydrated = await HydrateAsync(vault, changes, cancellationToken).ConfigureAwait(false);
 
         // When nothing came back the cursor must not move, or a concurrent write landing between
@@ -622,14 +630,30 @@ internal sealed class SyncService(
     /// <summary>The kind behind a change-log entry, or null for a type this server does not synchronise.</summary>
     private static IItemKind? KindForChange(ChangeEntityType type) => ItemKinds.For(ToWire(type));
 
-    private async Task<long> CurrentHeadAsync(Guid vaultId, CancellationToken cancellationToken) =>
+    /// <remarks>
-        await database.VaultChanges
+    /// Never below the vault's horizon: collecting a tombstone can take the newest entries with it, and a
+    /// head reported lower than a cursor the client already holds would rewind it under the horizon and
+    /// cost a full resync for nothing.
+    /// </remarks>
+    private async Task<long> CurrentHeadAsync(Guid vaultId, CancellationToken cancellationToken)
+    {
+        var head = await database.VaultChanges
             .Where(c => c.VaultId == vaultId)
             .OrderByDescending(c => c.Sequence)
             .Select(c => c.Sequence)
             .FirstOrDefaultAsync(cancellationToken)
             .ConfigureAwait(false);
 
+        return Math.Max(head, await HorizonAsync(vaultId, cancellationToken).ConfigureAwait(false));
+    }
+
+    /// <summary>The vault's horizon as the database holds it now, not as the request loaded it.</summary>
+    private Task<long> HorizonAsync(Guid vaultId, CancellationToken cancellationToken) =>
+        database.Vaults
+            .Where(v => v.Id == vaultId)
+            .Select(v => v.SyncHorizon)
+            .SingleAsync(cancellationToken);
+
     private static SyncPushResult Invalid(SyncPushOperation operation, string detail) =>
         new(
             operation.OperationId,

src/DodoSSH.Api/Features/Sync/TombstoneCollector.cs

@@ -0,0 +1,198 @@
+using DodoSSH.Api.Setup;
+using DodoSSH.Domain;
+using DodoSSH.Infrastructure;
+using Microsoft.EntityFrameworkCore;
+using Microsoft.Extensions.Options;
+
+namespace DodoSSH.Api.Features.Sync;
+
+/// <summary>
+/// Removes tombstones past their retention, and change-log entries a later one has superseded.
+/// </summary>
+/// <remarks>
+/// <para>
+/// ADR 0003 promised this and nothing did it: every deletion stayed a row for ever, and every edit stayed a
+/// change-log entry for ever, so the log grew without bound and a client starting from nothing walked every
+/// edit ever made to read the items that exist.
+/// </para>
+/// <para>
+/// <b>Superseded entries go at any time.</b> A pull returns entries after a cursor in order, so a client
+/// before both an item's old entry and its new one reads the new one, and a client between them already has
+/// the old one. Nothing reads history: a conflict resolves against the newest entry alone.
+/// </para>
+/// <para>
+/// <b>Tombstones go after the retention</b>, and taking them is the one change here a client can notice: a
+/// machine that last synced before the deletion would never hear of it. So each vault records the highest
+/// sequence collected from it — <see cref="Vault.SyncHorizon"/> — and a pull from a cursor below that is
+/// refused. The client answers by starting over, and a pull from nothing removes what the server no longer
+/// has; see the client's <c>SyncEngine</c>.
+/// </para>
+/// <para>
+/// Per vault under the same advisory lock a push takes, so a collection never interleaves with a write to
+/// the vault — and the items are read again under it, so one restored since it was found is left alone.
+/// </para>
+/// </remarks>
+internal sealed class TombstoneCollector(
+    DodoDbContext database,
+    IOptions<SyncOptions> syncOptions,
+    TimeProvider clock,
+    ILogger<TombstoneCollector> logger)
+{
+    /// <summary>Collects what is due, once.</summary>
+    /// <returns>How many tombstones and superseded entries went.</returns>
+    internal async Task<int> RunOnceAsync(CancellationToken cancellationToken)
+    {
+        var before = clock.GetUtcNow() - TimeSpan.FromDays(syncOptions.Value.TombstoneRetentionDays);
+
+        var collected = 0;
+
+        collected += await CollectAsync(db => db.Hosts, ChangeEntityType.SshHost, before, cancellationToken).ConfigureAwait(false);
+        collected += await CollectAsync(db => db.SshKeys, ChangeEntityType.SshKey, before, cancellationToken).ConfigureAwait(false);
+        collected += await CollectAsync(db => db.Credentials, ChangeEntityType.Credential, before, cancellationToken).ConfigureAwait(false);
+        collected += await CollectAsync(db => db.KnownHostKeys, ChangeEntityType.KnownHostKey, before, cancellationToken).ConfigureAwait(false);
+        collected += await CollectAsync(db => db.HostGroups, ChangeEntityType.HostGroup, before, cancellationToken).ConfigureAwait(false);
+        collected += await CollectAsync(db => db.Tags, ChangeEntityType.Tag, before, cancellationToken).ConfigureAwait(false);
+        collected += await CollectAsync(db => db.Snippets, ChangeEntityType.Snippet, before, cancellationToken).ConfigureAwait(false);
+        collected += await CollectAsync(db => db.ConnectionLog, ChangeEntityType.ConnectionLogEntry, before, cancellationToken).ConfigureAwait(false);
+        collected += await CollectAsync(db => db.ActivityLog, ChangeEntityType.ActivityLogEntry, before, cancellationToken).ConfigureAwait(false);
+        collected += await CollectAsync(db => db.ObjectStores, ChangeEntityType.ObjectStore, before, cancellationToken).ConfigureAwait(false);
+        collected += await CollectAsync(db => db.SpelunkKeys, ChangeEntityType.SpelunkKey, before, cancellationToken).ConfigureAwait(false);
+
+        collected += await CompactAsync(cancellationToken).ConfigureAwait(false);
+
+        SyncLog.Collected(logger, collected);
+
+        return collected;
+    }
+
+    private async Task<int> CollectAsync<T>(
+        Func<DodoDbContext, DbSet<T>> table,
+        ChangeEntityType type,
+        DateTimeOffset before,
+        CancellationToken cancellationToken)
+        where T : class, IVaultItem
+    {
+        var due = await table(database)
+            .Where(item => item.DeletedAtUtc != null && item.DeletedAtUtc < before)
+            .Select(item => new { item.VaultId, item.Id })
+            .ToListAsync(cancellationToken)
+            .ConfigureAwait(false);
+
+        var collected = 0;
+
+        foreach (var vault in due.GroupBy(item => item.VaultId))
+        {
+            var ids = vault.Select(item => item.Id).ToArray();
+
+            collected += await database.ExecuteFreshAsync(
+                [],
+                () => CollectFromVaultAsync(table, type, vault.Key, ids, before, cancellationToken))
+                .ConfigureAwait(false);
+        }
+
+        return collected;
+    }
+
+    private async Task<int> CollectFromVaultAsync<T>(
+        Func<DodoDbContext, DbSet<T>> table,
+        ChangeEntityType type,
+        Guid vaultId,
+        Guid[] ids,
+        DateTimeOffset before,
+        CancellationToken cancellationToken)
+        where T : class, IVaultItem
+    {
+        var transaction = await database.Database.BeginTransactionAsync(cancellationToken).ConfigureAwait(false);
+        await using var _ = transaction.ConfigureAwait(false);
+
+        await database.Database
+            .ExecuteSqlAsync($"SELECT pg_advisory_xact_lock(hashtextextended({vaultId.ToString()}, 0))", cancellationToken)
+            .ConfigureAwait(false);
+
+        // Read again under the lock: an item restored since it was found is no longer due.
+        var still = await table(database)
+            .Where(item => item.VaultId == vaultId
+                && ids.Contains(item.Id)
+                && item.DeletedAtUtc != null
+                && item.DeletedAtUtc < before)
+            .Select(item => new { item.Id, item.ChangeSequence })
+            .ToListAsync(cancellationToken)
+            .ConfigureAwait(false);
+
+        if (still.Count == 0)
+        {
+            return 0;
+        }
+
+        var going = still.Select(item => item.Id).ToArray();
+        var horizon = still.Max(item => item.ChangeSequence);
+
+        await table(database)
+            .Where(item => item.VaultId == vaultId && going.Contains(item.Id))
+            .ExecuteDeleteAsync(cancellationToken)
+            .ConfigureAwait(false);
+
+        await database.VaultChanges
+            .Where(change => change.VaultId == vaultId && change.EntityType == type && going.Contains(change.EntityId))
+            .ExecuteDeleteAsync(cancellationToken)
+            .ConfigureAwait(false);
+
+        await database.Vaults
+            .Where(v => v.Id == vaultId && v.SyncHorizon < horizon)
+            .ExecuteUpdateAsync(set => set.SetProperty(v => v.SyncHorizon, horizon), cancellationToken)
+            .ConfigureAwait(false);
+
+        await transaction.CommitAsync(cancellationToken).ConfigureAwait(false);
+
+        return still.Count;
+    }
+
+    /// <summary>Removes every change-log entry a later entry for the same item has superseded.</summary>
+    private Task<int> CompactAsync(CancellationToken cancellationToken) =>
+        database.Database.ExecuteSqlRawAsync(
+            "DELETE FROM " + DodoDbContext.SchemaName + ".sync_change AS older "
+            + "USING " + DodoDbContext.SchemaName + ".sync_change AS newer "
+            + "WHERE newer.vault_id = older.vault_id "
+            + "AND newer.entity_type = older.entity_type "
+            + "AND newer.entity_id = older.entity_id "
+            + "AND newer.sequence > older.sequence",
+            cancellationToken);
+}
+
+/// <summary>Runs the <see cref="TombstoneCollector"/> on a schedule, in a scope of its own each time.</summary>
+internal sealed class TombstoneCollectionService(
+    IServiceScopeFactory scopes,
+    IOptions<SyncOptions> syncOptions,
+    TimeProvider clock,
+    ILogger<TombstoneCollectionService> logger) : BackgroundService
+{
+    /// <inheritdoc />
+    protected override async Task ExecuteAsync(CancellationToken stoppingToken)
+    {
+        var interval = TimeSpan.FromMinutes(syncOptions.Value.TombstoneCollectionIntervalMinutes);
+
+        while (!stoppingToken.IsCancellationRequested)
+        {
+            try
+            {
+                await Task.Delay(interval, clock, stoppingToken).ConfigureAwait(false);
+
+                var scope = scopes.CreateAsyncScope();
+                await using var _ = scope.ConfigureAwait(false);
+
+                await scope.ServiceProvider.GetRequiredService<TombstoneCollector>()
+                    .RunOnceAsync(stoppingToken)
+                    .ConfigureAwait(false);
+            }
+            catch (OperationCanceledException) when (stoppingToken.IsCancellationRequested)
+            {
+                return;
+            }
+            catch (Exception exception) when (exception is not OutOfMemoryException)
+            {
+                // The next run tries again; nothing collected late is harmed by waiting.
+                SyncLog.CollectionFailed(logger, exception);
+            }
+        }
+    }
+}

src/DodoSSH.Api/Program.cs

@@ -48,6 +48,10 @@ builder.Services.AddSingleton<ICursorKeyProvider, CursorKeyProvider>();
 builder.Services.AddSingleton<VaultEventHub>();
 builder.Services.AddSingleton<IVaultEventPublisher, VaultEventFanOut>();
 
+// ADR 0003's tombstone collection, and the compaction of the change log that goes with it.
+builder.Services.AddScoped<TombstoneCollector>();
+builder.Services.AddHostedService<TombstoneCollectionService>();
+
 // The relay's services, its ticket key ring, and a shutdown long enough for its drain. See ADR 0004.
 builder.Services.AddDodoRelay();
 

src/DodoSSH.Api/Setup/DodoOptions.cs

@@ -289,6 +289,10 @@ public sealed class SyncOptions
     [Range(1, 3650)]
     public int TombstoneRetentionDays { get; set; } = 90;
 
+    /// <summary>How often the tombstone collector runs, in minutes.</summary>
+    [Range(1, 7 * 24 * 60)]
+    public int TombstoneCollectionIntervalMinutes { get; set; } = 6 * 60;
+
     /// <summary>
     /// Key used to sign sync cursors, base64.
     /// </summary>

src/DodoSSH.Domain/Vaults.cs

@@ -52,6 +52,16 @@ public sealed class Vault
     /// <summary>Soft-delete marker.</summary>
     public DateTimeOffset? DeletedAtUtc { get; set; }
 
+    /// <summary>
+    /// The highest change-log sequence whose tombstone has been collected from this vault.
+    /// </summary>
+    /// <remarks>
+    /// A cursor below it may have been issued before a deletion this server no longer records, so a pull
+    /// from it is refused and the client starts over — see <c>TombstoneCollector</c> and ADR 0003. Zero until
+    /// anything has been collected.
+    /// </remarks>
+    public long SyncHorizon { get; set; }
+
     /// <summary>Wrapped vault keys, one per recipient per generation.</summary>
     public ICollection<VaultKeyGrant> KeyGrants { get; } = [];
 

src/DodoSSH.Infrastructure/Migrations/20261002151534_AddVaultSyncHorizon.cs

@@ -0,0 +1,31 @@
+using Microsoft.EntityFrameworkCore.Migrations;
+
+#nullable disable
+
+namespace DodoSSH.Infrastructure.Migrations
+{
+    /// <inheritdoc />
+    public partial class AddVaultSyncHorizon : Migration
+    {
+        /// <inheritdoc />
+        protected override void Up(MigrationBuilder migrationBuilder)
+        {
+            migrationBuilder.AddColumn<long>(
+                name: "sync_horizon",
+                schema: "dodo",
+                table: "vault",
+                type: "bigint",
+                nullable: false,
+                defaultValue: 0L);
+        }
+
+        /// <inheritdoc />
+        protected override void Down(MigrationBuilder migrationBuilder)
+        {
+            migrationBuilder.DropColumn(
+                name: "sync_horizon",
+                schema: "dodo",
+                table: "vault");
+        }
+    }
+}

src/DodoSSH.Infrastructure/Migrations/DodoDbContextModelSnapshot.cs

@@ -750,6 +750,10 @@ namespace DodoSSH.Infrastructure.Migrations
                         .HasColumnType("boolean")
                         .HasColumnName("rekey_required");
 
+                    b.Property<long>("SyncHorizon")
+                        .HasColumnType("bigint")
+                        .HasColumnName("sync_horizon");
+
                     b.Property<Guid?>("TeamId")
                         .HasColumnType("uuid")
                         .HasColumnName("team_id");

tests/DodoSSH.Api.Tests/SyncEndpointTests.cs

@@ -1,5 +1,6 @@
 using System.Net;
 using System.Net.Http.Json;
+using DodoSSH.Api.Features.Sync;
 using DodoSSH.Contracts;
 using DodoSSH.Domain;
 using DodoSSH.Infrastructure;
@@ -612,6 +613,150 @@ public sealed class SyncEndpointTests(ApiFixture fixture)
         tombstone.PlaintextFields.ShouldBeNull();
     }
 
+    // ---- Tombstone collection (ADR 0003) ----
+
+    /// <remarks>
+    /// <para>
+    /// A tombstone past its retention goes, with its change-log entries — and a client whose cursor is from
+    /// before it is refused, because it would otherwise never hear of the deletion. A client already past it
+    /// carries on, and a pull from nothing simply no longer offers the item.
+    /// </para>
+    /// <para>
+    /// The retention is crossed by moving the deletion back in time rather than by waiting it out.
+    /// </para>
+    /// </remarks>
+    [Fact]
+    public async Task AnExpiredTombstone_IsCollected_AndACursorFromBeforeIt_IsRefused()
+    {
+        var (subject, vaultId) = await SeedUserWithVaultAsync();
+        var client = fixture.CreateClientFor(subject);
+
+        var create = NewCreateBatch();
+        var entityId = create.Operations[0].EntityId;
+        await client.PostContractAsync(PushUrl(vaultId), create);
+
+        var before = await PullAsync(client, vaultId, null);
+
+        await client.PostContractAsync(PushUrl(vaultId), new SyncPushRequest(
+        [
+            new SyncPushOperation(
+                Guid.CreateVersion7(), SyncEntityType.Host, entityId, SyncOperation.Delete,
+                ExpectedVersion: 1, Payload: null, PlaintextFields: null),
+        ]));
+
+        var after = await PullAsync(client, vaultId, before.NextCursor);
+
+        await AgeTheDeletionAsync(entityId);
+        await CollectAsync();
+
+        var refused = await client.PostContractAsync(PullUrl(vaultId), new SyncPullRequest(before.NextCursor, null, null));
+        refused.StatusCode.ShouldBe(HttpStatusCode.BadRequest);
+        (await refused.Content.ReadProblemAsync()).ShouldNotBeNull().Code.ShouldBe(ProblemCodes.InvalidCursor);
+
+        (await PullAsync(client, vaultId, after.NextCursor)).Changes.ShouldBeEmpty("a cursor past the deletion is fine");
+
+        (await PullAsync(client, vaultId, null)).Changes.ShouldNotContain(change => change.EntityId == entityId);
+    }
+
+    /// <remarks>
+    /// The other half of the retention: a deletion younger than it stays, entries and all, so a machine that
+    /// has been away a week still hears of it.
+    /// </remarks>
+    [Fact]
+    public async Task ATombstoneInsideItsRetention_IsKept()
+    {
+        var (subject, vaultId) = await SeedUserWithVaultAsync();
+        var client = fixture.CreateClientFor(subject);
+
+        var create = NewCreateBatch();
+        var entityId = create.Operations[0].EntityId;
+        await client.PostContractAsync(PushUrl(vaultId), create);
+
+        var before = await PullAsync(client, vaultId, null);
+
+        await client.PostContractAsync(PushUrl(vaultId), new SyncPushRequest(
+        [
+            new SyncPushOperation(
+                Guid.CreateVersion7(), SyncEntityType.Host, entityId, SyncOperation.Delete,
+                ExpectedVersion: 1, Payload: null, PlaintextFields: null),
+        ]));
+
+        await CollectAsync();
+
+        var later = await PullAsync(client, vaultId, before.NextCursor);
+
+        later.Changes.ShouldHaveSingleItem().Operation.ShouldBe(SyncOperation.Delete);
+    }
+
+    /// <remarks>
+    /// An item edited many times kept an entry per edit for ever, and a client starting from nothing walked
+    /// every one of them. After a collection only the newest is left — and a client that held a cursor from
+    /// between the edits still gets the item, because what it had not seen yet is exactly that newest entry.
+    /// </remarks>
+    [Fact]
+    public async Task SupersededEntries_AreCompactedAway_WithoutCostingAnyClientTheItem()
+    {
+        var (subject, vaultId) = await SeedUserWithVaultAsync();
+        var client = fixture.CreateClientFor(subject);
+
+        var create = NewCreateBatch();
+        var entityId = create.Operations[0].EntityId;
+        await client.PostContractAsync(PushUrl(vaultId), create);
+
+        var between = await PullAsync(client, vaultId, null);
+
+        await client.PostContractAsync(PushUrl(vaultId), new SyncPushRequest(
+        [
+            NewOperation(entityId, expectedVersion: 1, envelope: [2, 2, 2]),
+        ]));
+
+        await client.PostContractAsync(PushUrl(vaultId), new SyncPushRequest(
+        [
+            NewOperation(entityId, expectedVersion: 2, envelope: [3, 3, 3]),
+        ]));
+
+        await CollectAsync();
+
+        await using (var scope = fixture.CreateScope())
+        {
+            var database = scope.ServiceProvider.GetRequiredService<DodoDbContext>();
+
+            (await database.VaultChanges.CountAsync(c => c.EntityId == entityId, TestContext.Current.CancellationToken))
+                .ShouldBe(1);
+        }
+
+        (await PullAsync(client, vaultId, between.NextCursor)).Changes.ShouldHaveSingleItem().Version.ShouldBe(3);
+        (await PullAsync(client, vaultId, null)).Changes.Single(c => c.EntityId == entityId).Version.ShouldBe(3);
+    }
+
+    private static async Task<SyncPullResponse> PullAsync(HttpClient client, Guid vaultId, string? cursor)
+    {
+        var response = await client.PostContractAsync(PullUrl(vaultId), new SyncPullRequest(cursor, null, null));
+
+        response.StatusCode.ShouldBe(HttpStatusCode.OK);
+
+        return (await response.Content.ReadContractAsync<SyncPullResponse>()).ShouldNotBeNull();
+    }
+
+    /// <summary>Moves a deletion back past the retention, which is quicker than waiting ninety days.</summary>
+    private async Task AgeTheDeletionAsync(Guid entityId)
+    {
+        await using var scope = fixture.CreateScope();
+        var database = scope.ServiceProvider.GetRequiredService<DodoDbContext>();
+
+        var host = await database.Hosts.SingleAsync(h => h.Id == entityId, TestContext.Current.CancellationToken);
+        host.DeletedAtUtc = host.DeletedAtUtc!.Value.AddDays(-91);
+
+        await database.SaveChangesAsync(TestContext.Current.CancellationToken);
+    }
+
+    private async Task CollectAsync()
+    {
+        await using var scope = fixture.CreateScope();
+
+        await scope.ServiceProvider.GetRequiredService<TombstoneCollector>().RunOnceAsync(TestContext.Current.CancellationToken);
+    }
+
     // ---- Batch limits ----
 
     [Fact]