From 4ff5b1e5c6c65eafb1557e22bd2f89952f3b96a2 Mon Sep 17 00:00:00 2001 From: fabbrik <22822543+fabbrik@users.noreply.github.com> Date: Wed, 23 Sep 2026 13:01:44 -0300 Subject: [PATCH] Delete and expire library-owned experience data (story 4.5) Revocation stops reads; this adds the erasure. One authorized, atomic, scope-safe operation removes every payload-bearing trace of an experience across seven tables and leaves a payload-free tombstone that prevents the ID being written again. A retention sweep runs the same operation in bounded batches; default retention is indefinite and scheduling belongs to the host. Erasure is an authorized exception to the append-only guards, not a suspension of them. A new 0010 replaces the trigger functions so they recognise one transaction-scoped marker set only inside the purge function; 0006 is journaled and untouched, every ENABLE ALWAYS binding survives, and the guard stays armed for every other statement in every other session throughout. This replaces the documented runbook that disabled a trigger table-wide. It is an auditability mechanism, not a privilege boundary, and the documentation says so: a custom GUC is settable by any session and the guards already do not bind a role that can ALTER TABLE. What it buys is one code path, inside one transaction, with the guard never switched off and never left off across a failure. The purge functions are SECURITY DEFINER, so EXECUTE is revoked from PUBLIC and granted explicitly. Without that revoke a SELECT-only role could erase any record in any tenant. Writers that gate on deleted_at now take a key-share lock on the record row, so an in-flight embedding, grant or feedback write serializes against a purge instead of against a snapshot taken before it committed. experience_records gains DELETE and TRUNCATE guards: the purge updates that row into a tombstone and never deletes it, so nothing legitimate removes it, and a removal would orphan the audit trail and free the ID for reuse under another scope's grants. What erasure does not reach is stated rather than implied: backups, replicas, WAL, exported telemetry, external artifacts, dead index entries until VACUUM, and the previous row version in the heap, which still carries the erased text until the table is vacuumed. Also closes the cross-scope squat by mixing Scope into the derived experience ID -- a breaking change to record identity, taken now because it is free before release -- and injects TimeProvider into the stores that lacked one. Co-Authored-By: Claude Opus 5 (1M context) --- README.md | 59 +- .../ExperienceRecordStore.cs | 32 +- .../ExperienceFinalizationService.cs | 136 +- .../PostgresExperienceEmbeddingIndex.cs | 39 +- .../README.md | 23 +- .../AgentExperience.Storage.Postgres.csproj | 1 + .../ExperienceRecordValidator.cs | 66 + .../ExperienceRetention.cs | 124 ++ .../Migrations/0010_delete_and_expire.sql | 799 ++++++++ .../PostgresExperienceCandidateSource.cs | 12 +- .../PostgresExperienceGrantStore.cs | 117 ++ .../PostgresExperienceRecordSchema.cs | 36 + .../PostgresExperienceRecordStore.cs | 549 ++++- .../PostgresExperienceReuseFeedbackStore.cs | 124 +- .../README.md | 339 ++- .../ContractShapeTests.cs | 5 +- .../Diagnostics/ExperienceLoop.cs | 2 +- .../Diagnostics/ExperienceTelemetryTests.cs | 2 +- .../ExperienceFinalizationServiceTests.cs | 73 +- .../ExperienceFinalizationWiringTests.cs | 2 +- .../GoldenFailedTrialReport.txt | 4 +- .../GoldenNegativeControlReport.txt | 4 +- .../GoldenReport.txt | 4 +- .../GoldenTranscript.txt | 8 +- .../OfflineStoreTests.cs | 247 ++- .../PlainPostgresMigrationTests.cs | 88 +- .../PostgresDeletionTests.cs | 1826 +++++++++++++++++ .../PostgresFinalizationTests.cs | 4 +- .../PostgresFixture.cs | 41 + .../PostgresDeindexingTests.cs | 115 ++ .../TestWorld.cs | 40 +- 31 files changed, 4809 insertions(+), 112 deletions(-) create mode 100644 src/AgentExperience.Storage.Postgres/ExperienceRetention.cs create mode 100644 src/AgentExperience.Storage.Postgres/Migrations/0010_delete_and_expire.sql create mode 100644 tests/AgentExperience.Storage.Postgres.Tests/PostgresDeletionTests.cs diff --git a/README.md b/README.md index bfb199b..a6c0aa0 100644 --- a/README.md +++ b/README.md @@ -43,6 +43,7 @@ AgentExperience.NET records observable evidence (tool calls, results, errors, ve | Historical Reference injection into MAF: a context provider that retrieves, re-checks eligibility immediately before injecting, asks the host's risk policy, and injects one delimited, labeled block within record and byte limits — never throwing into the invocation | `AgentExperience.MicrosoftAgentFramework` | | Explicit sharing grants: an administrator the host names lets one named record be *read* by a sibling scope until it expires or is revoked; the grant and its audit event commit together, and reads honour it in SQL, never in application code. A grant's lifetime is bounded by a host-configured maximum, so there is no permanent grant | `AgentExperience.Abstractions`, `AgentExperience.Storage.Postgres` | | An optional access log answering "who read our team's experience, and when": one append-only row per record a grant *delivered*, naming that grant and the revision disclosed, written outside the read's own statement and batched per search, best-effort or fail-closed as the host chooses, with an owner-scoped reader for the trail | `AgentExperience.Abstractions`, `AgentExperience.Storage.Postgres` | +| Deleting and expiring library-owned data: one authorized, atomic, scope-safe erasure across seven tables leaving a payload-free tombstone, a bounded retention sweep the host schedules, and expired sharing grants collected with their events — with the append-only guards never disabled and the limits stated rather than overclaimed | `AgentExperience.Storage.Postgres`, `AgentExperience.Storage.Postgres.Vectors` | | Dependency-injection registration for each package, so a host wires capture, finalization, storage, indexing, and retrieval without knowing concrete types. Injection is the one piece the host constructs itself, because the resolver and risk decision are per-host | `AgentExperience.Core`, `AgentExperience.Storage.Postgres`, `AgentExperience.Storage.Postgres.Vectors` | ## Quick look @@ -286,11 +287,54 @@ otherwise rewrite history — not against an administrator who has decided to ta tamper-evidence beyond this should ship the log off-box, or own these tables with a role the application does not have. -**Because nothing can delete, purging is an explicit operator action.** The logs carry free-text `reason` and -`producer` that a host may have filled with personal data, and roadmap story 4.5 ("delete and expire library-owned -data") has not landed. Until it does, the tables' owner purges in one transaction — disable the trigger, delete -narrowly, re-enable it — as documented in `0006`'s own header, and reconciles `experience_records` afterwards, -because deleting an event does not move the projection. +**There is exactly one exception, and it is the subject of the next section.** Migration `0010` gives the guards a +transaction-scoped marker that one `SECURITY DEFINER` purge function sets, so erasing a record can remove the rows +that named it without any trigger ever being disabled. `UPDATE` and `TRUNCATE` stay refused unconditionally, in +every session, including the purging one. That replaces `0006`'s manual +`ALTER TABLE … DISABLE TRIGGER` runbook — which was table-wide, visible to every other connection in the pool, and +left the guard off if anything failed in between. + +## Deleting and expiring data + +Revocation stops reads. **Deletion removes payload.** One authorized, atomic, scope-safe operation erases every +payload-bearing trace of one experience across seven tables and leaves a payload-free tombstone behind: + +```csharp +var deleted = await store.DeleteAsync(hostAuthorization, scope, experienceId, cancellationToken); + +// Or on a schedule the host owns: this library ships no timer. +var sweep = await store.SweepExpiredAsync(hostAuthorization, scope, TimeSpan.FromDays(90), batchSize: 200, cancellationToken); +``` + +- **A tombstone, never a vanishing row.** What is retained is exactly the opaque `ExperienceId`, the six scope + fields, the revision, the deletion timestamp, a fixed tombstone status, and a fixed `TaskId` placeholder — + nothing else. Evidence, exposure rows, grants and their events, lifecycle history, and the embedding are + *removed*. The access trail (`experience_grant_access`) is deliberately kept: it carries no payload and is the + answer to "who read this before it was deleted". +- **A tombstone is terminal.** A late create, lifecycle commit, confidence submission, feedback write, index write, + or grant naming it is refused, never resurrected — and, within its own scope, a host can tell `Deleted` from + `NotFound`. Across scopes the two collapse: a foreign-scope delete is the same answer as one naming an ID that + never existed. Three of those refusals are enforced by the schema and cannot be worked around at all: the ID can + never be re-created, the tombstone can never be moved, and the record row can never be deleted or truncated. The + rest are predicates this library puts in its own statements — binding for everything that goes through the + library, not for raw SQL from another tool. The adapter README says which is which, table by table, rather than + claiming the stronger version of both. +- **Retention is indefinite by default.** A sweep runs only when a host passes a positive age, in bounded batches, + through the same delete. The library ships no timer, no background service, and no hosted service: scheduling + belongs to the host — **and a sweep matches one exact scope**, so a host with a tenant-wide policy has to + enumerate its own leaf scopes and sweep each one. A sweep of the tenant alone reports a clean `MoreRemain: + false` while every team-, agent- and user-scoped record stays put. +- **It is an auditability mechanism, not a privilege boundary.** The purge path buys one code path, one + transaction, and a guard that is never switched off — not protection from an administrator. A custom GUC is + settable by any session, and the guards still do not bind a role that can `ALTER TABLE`, which the application + role can. There is one real privilege boundary: `0010` revokes `EXECUTE` on both `SECURITY DEFINER` purge + functions from `PUBLIC`, because PostgreSQL's default would otherwise let any role that can connect erase any + tenant's record. +- **The limits are stated, including the uncomfortable one.** Backups, replicas, WAL, exported telemetry and + external artifacts are host-owned and out of reach — and *inside* this database the erased text survives in the + dead heap tuple until `VACUUM` reclaims it, which is a schedule nobody promised. The + [adapter README](src/AgentExperience.Storage.Postgres/README.md#deleting-and-expiring-data) carries that, the + retained list, the erasure order, and the full outcome table. **Upgrading an existing database.** `0006` adds every `CHECK` as `NOT VALID`, so it does not scan existing rows and cannot abort on a pre-`0006` `Superseded` event that has no replacement — one the public port accepted, because the @@ -407,8 +451,9 @@ against the history rather than reading that as "it never landed". prior and new counters, the evidence ID, the rule version, and the `Actor` — the principal the commit ran under, recorded by the store from the host's authorization and never from anything the caller put in the event. Read it through `GetHistoryAsync` like any other transition; `stored.Event.Confidence` is `null` for the events that carried none. -An *uncounted* submission has no event, by construction — the ledger row is its audit trail, and listing that ledger -arrives with roadmap story 4.5 along with its retention path. +An *uncounted* submission has no event, by construction — the ledger row is its audit trail. Listing that ledger is +still not a port operation; its retention is covered by a record's erasure, which removes every evidence row that +named it. ## Recording what reuse was worth diff --git a/src/AgentExperience.Abstractions/ExperienceRecordStore.cs b/src/AgentExperience.Abstractions/ExperienceRecordStore.cs index 768bfb5..85cb9e3 100644 --- a/src/AgentExperience.Abstractions/ExperienceRecordStore.cs +++ b/src/AgentExperience.Abstractions/ExperienceRecordStore.cs @@ -49,7 +49,12 @@ Task CreateAsync( /// The exact request scope to read within. /// The record to read. Must not be . /// Cancels the operation. - /// , , , or . + /// + /// , , + /// (from an implementation that supports erasure, when the + /// record was erased and the request scope is the one that owned it), + /// , or . + /// Task GetAsync( AuthorizationContext authorization, Scope scope, @@ -165,8 +170,9 @@ Task QueryAsync( /// , /// , /// (missing, or in another scope), - /// , , or - /// . + /// (the record was erased; a tombstone is terminal and nothing + /// is appended against one), , + /// , or . /// Task CommitLifecycleEventAsync( AuthorizationContext authorization, @@ -191,7 +197,12 @@ Task CommitLifecycleEventAsync( /// What the host has established the caller may do. /// The record whose history to read, the scope to read it within, the page bound, and the optional cursor. /// Cancels the operation. - /// (possibly with no events), , , or . + /// + /// (possibly with no events), + /// , (an erased + /// record has no history left to page), , or + /// . + /// Task GetHistoryAsync( AuthorizationContext authorization, ExperienceRecordHistoryQuery query, @@ -342,6 +353,19 @@ public enum ExperienceStoreOutcome /// when it had one, so the caller can tell "gone or not mine" from "no longer eligible". /// ReplacementNotAllowed, + + /// + /// The record named by this operation has been erased: its payload and every trace that named it + /// are gone, and a payload-free tombstone is all that remains under its ID. Nothing was written. + /// + /// It is distinct from so a host can tell "erased" from "never existed" + /// within its own scope. Across scopes the two collapse: a record in another scope is + /// whether or not it was ever erased, so this outcome reveals nothing the + /// caller did not already have authority over. A delete that erased a record and a delete naming + /// a record already erased both report it -- deleting twice is a success that touches nothing. + /// + /// + Deleted, } /// diff --git a/src/AgentExperience.Core/Finalization/ExperienceFinalizationService.cs b/src/AgentExperience.Core/Finalization/ExperienceFinalizationService.cs index 5a25db3..5e82daf 100644 --- a/src/AgentExperience.Core/Finalization/ExperienceFinalizationService.cs +++ b/src/AgentExperience.Core/Finalization/ExperienceFinalizationService.cs @@ -1,5 +1,8 @@ +using System.Buffers; +using System.Buffers.Binary; using System.Globalization; using System.Security.Cryptography; +using System.Text; using AgentExperience.Abstractions; using AgentExperience.Core.Capture; using AgentExperience.Core.Confidence; @@ -123,6 +126,9 @@ public sealed class ExperienceFinalizationService /// private static readonly Guid DerivationNamespace = new("0b6a8a3f-1c2d-4f5e-9a70-3d1c9f2b8e41"); + /// The fixed namespace, the run, and the purpose tag: the whole of the pre-scope derivation. + private const int PrefixLength = 33; + private const byte ExperienceIdTag = 1; private const byte InitialEventIdTag = 2; private const byte ReflectionIdTag = 3; @@ -199,9 +205,57 @@ public ExperienceFinalizationService( /// The budget this service gives the post-commit indexing hook. public TimeSpan IndexingTimeout { get; } - /// The finalizing issues, derived from the run so a retry re-derives the same ID. + /// + /// The finalizing in + /// issues, derived from both so a retry re-derives the same ID and no other + /// scope can derive it at all. + /// + /// + /// + /// Why the scope is mixed in. An is unique across + /// every scope -- it is the table's primary key -- while + /// 's conflict is deliberately scope-blind, so that a + /// taken ID reveals nothing about the scope that holds it. Derived from the run alone, the ID a run + /// will finalize under was predictable by anyone who knew the run ID, in any scope: writing a record + /// under it first left the real run unable to finalize, permanently and undiagnosably. Mixing the + /// scope in means a squatter must already be inside the scope it is blocking, where it could simply + /// write the record anyway. + /// + /// + /// Every scope field takes part, each length-prefixed, so no two different scopes can hash to the + /// same input by rearranging where one field ends and the next begins -- ("a", "bc") and ("ab", "c") + /// are different scopes and derive different IDs. + /// + /// + /// This replaces a one-argument ExperienceIdFor(Guid), with no compatible overload. The + /// library is pre-1.0 and unpublished, so the break costs nothing externally, and an + /// [Obsolete] overload could not have been kept honestly: it would have to derive the old, + /// squattable ID, which is the defect. A caller that had one updates it by passing the same + /// it finalizes the run under. Nothing persisted needs migrating either, because + /// a record's ID is stored, never re-derived from a run. + /// + /// + /// An erased record's run can never be finalized again. The derivation is deterministic, so a + /// re-run of finalization for the same run in the same scope derives the same ID, collides with the + /// tombstone that erasure left under it, and stops -- permanently. That is deliberate: a record was + /// deleted, and re-finalizing the run it came from would recreate exactly what the deletion removed. + /// It is worth naming that this is the same *shape* of dead end that mixing the scope in just closed, + /// and worth naming what makes it different: the squat was reachable from any scope and + /// undiagnosable, because 's conflict is deliberately + /// scope-blind. This one is reachable only by the scope that owns the record, and that scope can see + /// exactly why -- + /// answers for its own tombstone. A permanent dead end its owner + /// can diagnose is a different thing from a permanent dead end nobody can. + /// + /// /// The captured run. - public static Guid ExperienceIdFor(Guid runId) => Derive(runId, ExperienceIdTag); + /// The scope the record will be created in. + /// is . + public static Guid ExperienceIdFor(Guid runId, Scope scope) + { + ArgumentNullException.ThrowIfNull(scope); + return Derive(runId, ExperienceIdTag, scope); + } /// The of the record's initial event, derived from the run so a retry cannot commit a second initial confirmation. /// The captured run. @@ -456,7 +510,7 @@ private async Task FinalizeCoreAsync( // Stage 5 -- Create the record, as a Candidate. Attempts are copied unchanged: capture already // rejected anything unsafe, and finalization never sanitizes. var record = new ExperienceRecord( - ExperienceId: ExperienceIdFor(run.RunId), + ExperienceId: ExperienceIdFor(run.RunId, run.Scope), SourceRunId: run.RunId, Scope: run.Scope, TaskId: run.TaskId, @@ -922,23 +976,79 @@ private static DateTimeOffset TruncateToMicroseconds(DateTimeOffset value) } /// - /// Derives a stable identifier from a run ID and a per-purpose tag: SHA-256 over a fixed - /// namespace, the run ID, and the tag, stamped with the RFC 9562 custom version (8) and variant. - /// Same run in, same identifiers out -- which is what makes replaying finalization safe. + /// Derives a stable identifier from a run ID, a per-purpose tag, and -- for a record ID -- the scope + /// the record will live in: SHA-256 over a fixed namespace, the run ID, the tag, and each scope field + /// length-prefixed, stamped with the RFC 9562 custom version (8) and variant. Same inputs in, same + /// identifier out, which is what makes replaying finalization safe. + /// + /// The scope takes part for the record ID only. The reflection ID is carried inside the + /// record's own payload and is unique by construction once the record ID is. The initial event ID is + /// the one remaining run-derived identifier that is globally unique across scopes: a writer in + /// another scope that commits an event under it first makes this run's initial commit a + /// , which is the same shape of dead end mixing the + /// scope into the record ID just closed. It is left as it is deliberately rather than by oversight -- + /// the story that changed this derivation changed exactly what it set out to -- and is recorded as + /// open work rather than described here as solved. + /// /// - private static Guid Derive(Guid runId, byte tag) + private static Guid Derive(Guid runId, byte tag, Scope? scope = null) { - Span input = stackalloc byte[33]; - DerivationNamespace.TryWriteBytes(input[..16], bigEndian: true, out _); - runId.TryWriteBytes(input.Slice(16, 16), bigEndian: true, out _); - input[32] = tag; - Span hash = stackalloc byte[32]; - SHA256.HashData(input, hash); + + if (scope is null) + { + Span input = stackalloc byte[PrefixLength]; + WritePrefix(input, runId, tag); + SHA256.HashData(input, hash); + } + else + { + var input = new ArrayBufferWriter(PrefixLength + 96); + WritePrefix(input.GetSpan(PrefixLength), runId, tag); + input.Advance(PrefixLength); + + AppendScopeField(input, scope.TenantId); + AppendScopeField(input, scope.ApplicationId); + AppendScopeField(input, scope.ProjectId); + AppendScopeField(input, scope.TeamId); + AppendScopeField(input, scope.AgentId); + AppendScopeField(input, scope.UserId); + + SHA256.HashData(input.WrittenSpan, hash); + } var id = hash[..16]; id[6] = (byte)((id[6] & 0x0F) | 0x80); id[8] = (byte)((id[8] & 0x3F) | 0x80); return new Guid(id, bigEndian: true); } + + private static void WritePrefix(Span input, Guid runId, byte tag) + { + DerivationNamespace.TryWriteBytes(input[..16], bigEndian: true, out _); + runId.TryWriteBytes(input.Slice(16, 16), bigEndian: true, out _); + input[32] = tag; + } + + /// + /// One scope field, tagged present or absent and length-prefixed when present. An absent optional + /// field is deliberately not the empty string, and the length keeps two adjacent fields from being + /// re-divided: ("a", "bc") and ("ab", "c") are different scopes and must derive different IDs. + /// + private static void AppendScopeField(ArrayBufferWriter input, string? field) + { + if (field is null) + { + input.GetSpan(1)[0] = 0; + input.Advance(1); + return; + } + + var byteCount = Encoding.UTF8.GetByteCount(field); + var span = input.GetSpan(5 + byteCount); + span[0] = 1; + BinaryPrimitives.WriteInt32BigEndian(span[1..5], byteCount); + Encoding.UTF8.GetBytes(field, span[5..]); + input.Advance(5 + byteCount); + } } diff --git a/src/AgentExperience.Storage.Postgres.Vectors/PostgresExperienceEmbeddingIndex.cs b/src/AgentExperience.Storage.Postgres.Vectors/PostgresExperienceEmbeddingIndex.cs index a994545..a2e199f 100644 --- a/src/AgentExperience.Storage.Postgres.Vectors/PostgresExperienceEmbeddingIndex.cs +++ b/src/AgentExperience.Storage.Postgres.Vectors/PostgresExperienceEmbeddingIndex.cs @@ -131,7 +131,15 @@ public sealed class PostgresExperienceEmbeddingIndex : IExperienceEmbeddingIndex "@model_id, @dimension, @content_hash, @source_revision, CAST(@embedding AS vector), @now, @now " + $"FROM {PostgresExperienceRecordStore.Table} r " + $"WHERE r.experience_id = @experience_id AND {PostgresExperienceRecordStore.RecordScopePredicate} " + + // An erased record is never indexed: the erasure removes its vector, and an in-flight write that + // landed afterwards would put a derived copy of a deleted record back into the table. The lock is + // what makes that true under concurrency rather than only in the quiet case -- a stored vector is + // a searchable derivative of exactly the summary and lesson the erasure was asked to destroy, so + // this write must serialize against the purge and not against a snapshot it has already + // invalidated. See PostgresExperienceRecordStore.RecordKeyShareLock. + $"AND {PostgresExperienceRecordStore.RecordLivePredicate} " + "AND r.revision = @source_revision " + + $"{PostgresExperienceRecordStore.RecordKeyShareLock} " + "ON CONFLICT (experience_id) DO UPDATE SET " + "model_id = EXCLUDED.model_id, dimension = EXCLUDED.dimension, content_hash = EXCLUDED.content_hash, " + "source_revision = EXCLUDED.source_revision, embedding = EXCLUDED.embedding, updated_at = EXCLUDED.updated_at " + @@ -152,7 +160,10 @@ public sealed class PostgresExperienceEmbeddingIndex : IExperienceEmbeddingIndex /// private const string ProbeRevisionSql = $"SELECT r.revision FROM {PostgresExperienceRecordStore.Table} r " + - $"WHERE r.experience_id = @experience_id AND {PostgresExperienceRecordStore.RecordScopePredicate}"; + $"WHERE r.experience_id = @experience_id AND {PostgresExperienceRecordStore.RecordScopePredicate} " + + // A tombstone is reported the way a record in another scope is: Missing, never Stale. There is + // no revision to retry against, because no revision of an erased record can ever be indexed. + $"AND {PostgresExperienceRecordStore.RecordLivePredicate}"; /// /// One statement, so a record's revision, the summary read at that revision, and the descriptor of @@ -166,6 +177,7 @@ public sealed class PostgresExperienceEmbeddingIndex : IExperienceEmbeddingIndex $"FROM {PostgresExperienceRecordStore.Table} r " + $"LEFT JOIN {Table} e ON e.experience_id = r.experience_id " + $"WHERE {PostgresExperienceRecordStore.RecordScopePredicate} " + + $"AND {PostgresExperienceRecordStore.RecordLivePredicate} " + // The search's own predicates, applied here too: a record whose vector could never be returned // is never embedded, so its summary and lesson never leave the database for a third party. "AND r.status = ANY(@statuses) AND r.reuse_confidence >= @min_confidence"; @@ -192,18 +204,18 @@ public sealed class PostgresExperienceEmbeddingIndex : IExperienceEmbeddingIndex /// private static readonly string CompatibilityProbeExactSql = $"SELECT EXISTS (SELECT 1 FROM {Table} e JOIN {PostgresExperienceRecordStore.Table} r ON r.experience_id = e.experience_id " + - $"WHERE {ExactJoinScopePredicate} " + + $"WHERE {ExactJoinScopePredicate} AND {PostgresExperienceRecordStore.RecordLivePredicate} " + "AND r.status = ANY(@statuses) AND r.reuse_confidence >= @min_confidence), " + $"EXISTS (SELECT 1 FROM {Table} e JOIN {PostgresExperienceRecordStore.Table} r ON r.experience_id = e.experience_id " + - $"WHERE {ExactJoinScopePredicate} " + + $"WHERE {ExactJoinScopePredicate} AND {PostgresExperienceRecordStore.RecordLivePredicate} " + "AND r.status = ANY(@statuses) AND r.reuse_confidence >= @min_confidence AND e.model_id = @model_id)"; private static readonly string CompatibilityProbeSql = $"SELECT EXISTS (SELECT 1 FROM {Table} e JOIN {PostgresExperienceRecordStore.Table} r ON r.experience_id = e.experience_id " + - $"WHERE {ReadableJoinScopePredicate} " + + $"WHERE {ReadableJoinScopePredicate} AND {PostgresExperienceRecordStore.RecordLivePredicate} " + "AND r.status = ANY(@statuses) AND r.reuse_confidence >= @min_confidence), " + $"EXISTS (SELECT 1 FROM {Table} e JOIN {PostgresExperienceRecordStore.Table} r ON r.experience_id = e.experience_id " + - $"WHERE {ReadableJoinScopePredicate} " + + $"WHERE {ReadableJoinScopePredicate} AND {PostgresExperienceRecordStore.RecordLivePredicate} " + "AND r.status = ANY(@statuses) AND r.reuse_confidence >= @min_confidence AND e.model_id = @model_id)"; private static readonly IReadOnlyList NoErrors = []; @@ -218,6 +230,8 @@ public sealed class PostgresExperienceEmbeddingIndex : IExperienceEmbeddingIndex private readonly ExperienceGrantAuditing? _auditing; + private readonly TimeProvider _timeProvider; + /// Creates an embedding index over a host-owned data source. The index never disposes it. /// The Npgsql data source to open connections from. /// @@ -230,16 +244,26 @@ public sealed class PostgresExperienceEmbeddingIndex : IExperienceEmbeddingIndex /// is a disclosure; the whole search's rows are written in one statement. -- /// the default -- switches auditing off entirely. /// + /// + /// The clock this index stamps a stored vector's created_at and updated_at from -- + /// its own reading of when the row landed, never a caller's. Defaults to + /// . It is here for the same reason the record and feedback stores + /// took one in story 4.5: a store that stamps rows from DateTimeOffset.UtcNow cannot be + /// driven to a known instant by a test, and every other timestamp this library writes is now + /// controllable. + /// /// is . public PostgresExperienceEmbeddingIndex( NpgsqlDataSource dataSource, Action? onGrantsUnavailable = null, - ExperienceGrantAuditing? auditing = null) + ExperienceGrantAuditing? auditing = null, + TimeProvider? timeProvider = null) { ArgumentNullException.ThrowIfNull(dataSource); _dataSource = dataSource; _grants = new PostgresGrantSupport(onGrantsUnavailable); _auditing = auditing; + _timeProvider = timeProvider ?? TimeProvider.System; } /// @@ -281,7 +305,7 @@ public async Task WriteAsync( parameters.Add(new NpgsqlParameter("content_hash", NpgsqlDbType.Text) { TypedValue = write.Descriptor.ContentHash }); parameters.Add(new NpgsqlParameter("source_revision", write.Descriptor.SourceRevision)); parameters.Add(new NpgsqlParameter("embedding", NpgsqlDbType.Text) { TypedValue = ToVectorLiteral(write.Vector) }); - parameters.Add(new NpgsqlParameter("now", ToStoredTimestamp(DateTimeOffset.UtcNow))); + parameters.Add(new NpgsqlParameter("now", ToStoredTimestamp(_timeProvider.GetUtcNow()))); written = await command.ExecuteNonQueryAsync(cancellationToken).ConfigureAwait(false); } @@ -596,6 +620,7 @@ private static string SearchSql(int dimension, bool readable) // what makes ix_experience_embeddings_scope_model usable (see EmbeddingScopePredicate). // An active grant is the alternative to that exact match, decided in SQL like the rest. $"WHERE {scope} " + + $"AND {PostgresExperienceRecordStore.RecordLivePredicate} " + "AND r.status = ANY(@statuses) " + "AND r.reuse_confidence >= @min_confidence " + "AND e.model_id = @model_id " + diff --git a/src/AgentExperience.Storage.Postgres.Vectors/README.md b/src/AgentExperience.Storage.Postgres.Vectors/README.md index 929b8ca..538d878 100644 --- a/src/AgentExperience.Storage.Postgres.Vectors/README.md +++ b/src/AgentExperience.Storage.Postgres.Vectors/README.md @@ -167,8 +167,27 @@ other than `Removed` or `NotIndexed` is a work item for the host — record the `RemoveAsync` again later. That includes `Denied`, which reports `IsRetryable: false` because repeating the *same* call changes nothing; it needs a different authorization, not another attempt. -Deleting the record itself needs no removal at all — `0004`'s `ON DELETE CASCADE` means an embedding can never -outlive the record it describes. +**Erasing the record removes its vector too, from the other package's transaction.** `DeleteAsync` in +`AgentExperience.Storage.Postgres` leaves a payload-free tombstone rather than deleting the record row, so +`0004`'s `ON DELETE CASCADE` never fires — the base package's purge function deletes the embedding explicitly +instead, guarded by `to_regclass` so a deployment without this package simply skips the step. Nothing here has to +be called, and nothing here is depended on. Afterwards the tombstone can never be indexed again: a write against +it reports `Missing` rather than `Stale`, because no revision of an erased record can ever be indexed, and +`ScanAsync` does not offer it, because a tombstone has no summary and no lesson to embed. + +`WriteAsync` takes `FOR KEY SHARE` on the record row in the same statement that checks it is not a tombstone, so a +write already in flight when an erasure commits is parked against the purge and re-checks when it is released, +rather than landing afterwards. That matters more here than anywhere else: a stored vector is a searchable +derivative of exactly the summary and lesson the erasure was asked to destroy, so a write that slipped through +would put a queryable copy of erased content back into the database. + +**What the erasure leaves behind in this package.** Dead entries stay in the HNSW index until `VACUUM` reclaims +them. Those entries point at heap tuples that are themselves dead, so a query cannot return them — but be precise +about the two halves, because they are not the same: the *index* entry cannot return anything, while the *heap* +tuple it points at is still the row, vector and all, until `VACUUM` reclaims it. A deployment with an erasure +deadline has to `VACUUM agent_experience.experience_embeddings` itself rather than wait for autovacuum, and needs +`VACUUM FULL` or a storage-level guarantee if it must also defeat forensic recovery of freed pages. The base +package's README says the same about the record table's own heap, at more length. ## Re-indexing diff --git a/src/AgentExperience.Storage.Postgres/AgentExperience.Storage.Postgres.csproj b/src/AgentExperience.Storage.Postgres/AgentExperience.Storage.Postgres.csproj index 7c8ec3f..6923469 100644 --- a/src/AgentExperience.Storage.Postgres/AgentExperience.Storage.Postgres.csproj +++ b/src/AgentExperience.Storage.Postgres/AgentExperience.Storage.Postgres.csproj @@ -35,6 +35,7 @@ + diff --git a/src/AgentExperience.Storage.Postgres/ExperienceRecordValidator.cs b/src/AgentExperience.Storage.Postgres/ExperienceRecordValidator.cs index 12b8306..ba678b9 100644 --- a/src/AgentExperience.Storage.Postgres/ExperienceRecordValidator.cs +++ b/src/AgentExperience.Storage.Postgres/ExperienceRecordValidator.cs @@ -74,6 +74,72 @@ public static IReadOnlyList ValidateGet(Scope scope, Guid return errors; } + /// + /// Validates an erasure: the scope, the record, and the optional expected revision. A negative + /// revision is refused rather than treated as "any", because a caller that computed one is asking + /// for something it did not mean -- and the thing it is asking for here is destructive. + /// + public static IReadOnlyList ValidateDelete(Scope scope, Guid experienceId, long? expectedRevision) + { + var errors = new List(); + if (experienceId == Guid.Empty) + { + errors.Add(new("ExperienceId", "must not be an empty GUID.")); + } + + if (expectedRevision is < 0) + { + errors.Add(new("ExpectedRevision", "must not be negative.")); + } + + ValidateScope(scope, "Scope", errors); + return errors; + } + + /// + /// Validates a retention sweep: the scope, the age, and the batch bound. A non-positive age is + /// refused rather than read as "delete everything": retention is indefinite until a host names a + /// span, and a zero or negative one is the shape a misconfigured setting takes. + /// + public static IReadOnlyList ValidateRetentionSweep(Scope scope, TimeSpan retentionAge, int batchSize) + { + var errors = new List(); + + if (retentionAge <= TimeSpan.Zero) + { + errors.Add(new("RetentionAge", "must be strictly positive; there is no retention age that means 'delete everything'.")); + } + + if (batchSize is < PostgresExperienceRecordStore.MinSweepBatchSize or > PostgresExperienceRecordStore.MaxSweepBatchSize) + { + errors.Add(new( + "BatchSize", + $"must be between {PostgresExperienceRecordStore.MinSweepBatchSize} and {PostgresExperienceRecordStore.MaxSweepBatchSize}.")); + } + + ValidateScope(scope, "Scope", errors); + return errors; + } + + /// + /// Validates an expired-grant purge: the owner scope and the batch bound. There is no age + /// parameter, because a grant carries its own: it is collected once its stored expiry has passed. + /// + public static IReadOnlyList ValidateGrantPurge(Scope recordScope, int batchSize) + { + var errors = new List(); + + if (batchSize is < PostgresExperienceRecordStore.MinSweepBatchSize or > PostgresExperienceRecordStore.MaxSweepBatchSize) + { + errors.Add(new( + "BatchSize", + $"must be between {PostgresExperienceRecordStore.MinSweepBatchSize} and {PostgresExperienceRecordStore.MaxSweepBatchSize}.")); + } + + ValidateScope(recordScope, "RecordScope", errors); + return errors; + } + /// /// Validates a bounded history read: the scope, the record, the page bound, and the optional keyset /// cursor. A negative cursor is rejected rather than treated as "from the beginning", because a diff --git a/src/AgentExperience.Storage.Postgres/ExperienceRetention.cs b/src/AgentExperience.Storage.Postgres/ExperienceRetention.cs new file mode 100644 index 0000000..c01961e --- /dev/null +++ b/src/AgentExperience.Storage.Postgres/ExperienceRetention.cs @@ -0,0 +1,124 @@ +using AgentExperience.Abstractions; + +namespace AgentExperience.Storage.Postgres; + +/// +/// The result of . +/// +/// +/// These results live in this package rather than in AgentExperience.Abstractions on purpose: +/// erasure is a capability of this adapter, not of the port. Core +/// never deletes, and a port method would oblige every implementation -- including the in-memory +/// doubles hosts write for their tests -- to promise an erasure it cannot perform. +/// +/// +/// when the record was erased, or was already a tombstone; +/// when an expected revision was given and the record +/// has moved past it; when no record with that ID is in the +/// requested scope -- including when it is in another one; or +/// before any storage was touched. +/// +/// +/// The tombstone's on +/// -- erasure advances it once, exactly as a lifecycle +/// commit does -- or the record's current revision on +/// , so the caller can retry against it. Otherwise 0. +/// +/// Every validation error when is ; otherwise empty. +public sealed record ExperienceRecordDeleteResult( + ExperienceStoreOutcome Outcome, + long Revision, + IReadOnlyList Errors); + +/// +/// The result of one call to . +/// +/// +/// when the sweep ran -- including when it found nothing to +/// erase -- or or +/// before any storage was touched. +/// +/// How many records this call erased. Zero is a normal answer. +/// +/// Whether another call with the same arguments would find more records past the cutoff. It is what lets +/// a host's own scheduler drive a long sweep in bounded batches without this library owning a timer. +/// +/// Every validation error when is ; otherwise empty. +/// +/// Whether the batch stopped early, leaving records that were past the cutoff untouched. It is +/// for every sweep that ran to the end of its batch, including one that found +/// nothing. When it is , still counts exactly the +/// records this call erased -- erasure is per record and per transaction, so a stopped sweep leaves +/// every record it reached wholly erased -- and is , +/// because at least the record it stopped on is still there. +/// +public sealed record ExperienceRetentionSweepResult( + ExperienceStoreOutcome Outcome, + int DeletedCount, + bool MoreRemain, + IReadOnlyList Errors, + bool Interrupted = false); + +/// +/// A retention sweep that was interrupted by a storage failure part-way through its batch, carrying how +/// much of the batch was irreversibly erased before it stopped. +/// +/// +/// +/// Erasure is the one operation this library cannot undo, and the count is the only thing a compliance +/// log could record about a sweep that failed half-way. A bare +/// would throw that number away, so this carries it -- while +/// still being an , so a host that already catches the general +/// case is unaffected and does not have to learn a new type to stay correct. +/// +/// +/// Caller cancellation does not raise this: a cancelled sweep returns its +/// with +/// set, because stopping between records is a +/// normal, expected way to run a sweep and the host asked for it. +/// +/// +public sealed class ExperienceRetentionSweepInterruptedException : ExperienceStoreException +{ + /// Creates the exception around the work the sweep had already done. + /// What the sweep erased before it stopped. + /// The storage failure that stopped it. + /// is . + public ExperienceRetentionSweepInterruptedException(ExperienceRetentionSweepResult partial, Exception innerException) + : base("An Experience Record retention sweep was interrupted by a storage infrastructure error; " + + "the records it had already erased are erased.", innerException) + { + ArgumentNullException.ThrowIfNull(partial); + Partial = partial; + } + + /// + /// What the sweep erased before it stopped, with + /// set. Its + /// is exact. + /// + public ExperienceRetentionSweepResult Partial { get; } +} + +/// +/// The result of one call to . +/// +/// +/// It reports an rather than an +/// : that enum's vocabulary is issue, revoke, and read, and a purge +/// is none of the three. Calling a purge Revoked would put the word for "access ended, trail +/// kept" on the one operation that removes the trail. +/// +/// +/// when the purge ran -- including when it found nothing -- +/// or or before +/// any storage was touched. +/// +/// How many grant rows this call removed, with their audit events. +/// Whether another call with the same arguments would find more. +/// Every validation error when is ; otherwise empty. +public sealed record ExperienceGrantPurgeResult( + ExperienceStoreOutcome Outcome, + int PurgedCount, + bool MoreRemain, + IReadOnlyList Errors); diff --git a/src/AgentExperience.Storage.Postgres/Migrations/0010_delete_and_expire.sql b/src/AgentExperience.Storage.Postgres/Migrations/0010_delete_and_expire.sql new file mode 100644 index 0000000..876f5f7 --- /dev/null +++ b/src/AgentExperience.Storage.Postgres/Migrations/0010_delete_and_expire.sql @@ -0,0 +1,799 @@ +-- AgentExperience.NET: deletion and retention -- payload erasure with a payload-free tombstone (Story 4.5). +-- Applied by ExperienceSchemaMigrator and recorded in agent_experience.schema_versions. +-- Every statement is idempotent on purpose, matching 0001-0009, so a database whose schema was applied by +-- hand can still be journaled. Do not edit this script once it has been journaled anywhere; add the +-- next-numbered script instead. +-- +-- 0006's header pointed here: "until the library ships a purge path (roadmap story 4.5 ... which will need +-- a SECURITY DEFINER purge function or time-partitioned logs -- this script cannot be edited once +-- journaled)". This is that script. 0006 is untouched; its trigger functions are replaced in place with +-- CREATE OR REPLACE, so every ENABLE ALWAYS binding it created survives unchanged and no table is ever +-- left unguarded for an instant. +-- +-- WHAT DELETION IS HERE. Not a row vanishing: payload erasure plus a tombstone. The experience_records row +-- survives, carrying only the opaque experience_id, the six scope columns, revision, deleted_at, status, +-- and a fixed non-blank task_id placeholder. Everything else that named the record -- its evidence, its +-- exposure rows, its grants and their audit events, its lifecycle history, its embedding -- is REMOVED. +-- The tombstone is what makes the ID unusable afterwards: a create collides with it, and every other write +-- path refuses it. +-- +-- WHICH REFUSALS THE SCHEMA ENFORCES, AND WHICH THE ADAPTER DOES. "Every other write path refuses a +-- tombstone" is true of this library's write paths, and it is worth saying exactly where the rule lives, +-- because the two are not the same strength: +-- * SCHEMA-ENFORCED, so raw SQL cannot get round them: recreating the record (the primary key collides +-- with the surviving tombstone row); any UPDATE of a tombstone (enforce_record_projection below +-- refuses it, marker or not); setting, clearing or moving deleted_at outside the purge; a tombstone +-- row whose payload or task_id is not the erased shape (the tombstone-shape CHECK); and deleting or +-- truncating experience_records at all (reject_record_removal below). +-- * ADAPTER-ENFORCED, by a predicate the store puts in its own statements, and therefore only binding +-- for callers who go through this library: a lifecycle event or confidence-evidence row naming a +-- tombstone, an embedding write, a reuse-feedback exposure, and a sharing grant. Raw SQL can still +-- INSERT any of those rows against a tombstoned ID; there is no foreign key to experience_records on +-- any of those tables, deliberately (0002, 0005, 0007, 0008), and adding one now would rewrite four +-- journaled tables' shapes for this one rule. +-- * The adapter-enforced predicates are locked, not merely read: every one of them takes +-- FOR KEY SHARE on the record row, so a writer that starts before a purge commits is parked against +-- the purge's FOR UPDATE and re-checks the tombstone when it is released, instead of deciding +-- against a snapshot the purge has already invalidated. +-- +-- WHAT IS RETAINED AFTER A DELETE, EXHAUSTIVELY: +-- experience_id, tenant_id, application_id, project_id, team_id, agent_id, user_id, +-- revision, deleted_at, status (the fixed literal 'Deleted'), task_id (the fixed literal '(deleted)'), +-- payload_version. +-- Nothing else. payload_version is on the list rather than described as an exception to it: it describes +-- the (now empty) payload envelope's shape and says nothing about the record, but it does survive, and a +-- list that called itself exhaustive while omitting it would be wrong. source_run_id is zeroed, payload +-- becomes '{}'::jsonb, reuse_confidence and both counters become 0, and created_at and updated_at are set +-- to deleted_at -- a tombstone's only timestamp is the moment it was erased. search_vector is +-- GENERATED ALWAYS from task_id and two payload fields (0003), so erasing the payload and replacing +-- task_id regenerates it to hold only the placeholder -- the erasure of searchable text is automatic and +-- needs no separate index maintenance. +-- +-- experience_grant_access rows are DELIBERATELY NOT DELETED. They name a grant and a principal and carry +-- no record payload; they are the answer to "who read this before it was deleted", which is exactly the +-- question a deletion makes urgent. The purge marker below does not admit a delete on that table at all. +-- +-- HOW THE APPEND-ONLY GUARDS STAY ARMED. Erasure needs DELETE on five append-only tables. The runbook +-- 0006 documented -- ALTER TABLE ... DISABLE TRIGGER, DELETE, ENABLE ALWAYS TRIGGER -- is replaced rather +-- than automated, because disabling a trigger is table-wide and session-independent: for the length of +-- that window *every other connection in the pool* can rewrite the audit log, and a failure between +-- disable and re-enable leaves the guard off afterwards. Instead the guards themselves learn one +-- transaction-scoped marker: +-- +-- SET LOCAL agent_experience.purge_authorized = 'on' +-- +-- set only inside agent_experience.purge_experience_record below, and read by +-- agent_experience.purge_authorized(). The marker is invisible to every other session, it dies with the +-- transaction, and -- because the function declares a SET clause for the same variable -- it dies at +-- function exit even if the caller's transaction runs on. The guards keep refusing UPDATE and TRUNCATE +-- unconditionally, on every table, in every session, including the purging one. +-- +-- WHO MAY CALL THE PURGE FUNCTIONS. Both are SECURITY DEFINER, so they run with the owner's rights, and +-- PostgreSQL grants EXECUTE on a new function to PUBLIC by default. Left at the default that would make +-- them a universally callable erasure primitive: any role that can connect -- including a SELECT-only +-- reporting role explicitly denied DELETE and UPDATE on every table -- could read an experience_id and a +-- scope out of experience_records and erase that record, in any tenant. That is a privilege escalation in +-- the opposite direction from the one this script is otherwise careful about, so EXECUTE is revoked from +-- PUBLIC and granted explicitly, at the bottom of this script, to the role applying it -- which is the +-- role that owns these tables and runs the application. A deployment whose application role is not the +-- migrating role must grant it EXECUTE itself, once, and should grant it to nothing else: +-- +-- GRANT EXECUTE ON FUNCTION agent_experience.purge_experience_record( +-- uuid, text, text, text, text, text, text, bigint, timestamptz) TO ; +-- GRANT EXECUTE ON FUNCTION agent_experience.purge_expired_grants( +-- text, text, text, text, text, text, timestamptz, integer) TO ; +-- +-- THIS IS AN AUDITABILITY MECHANISM, NOT A PRIVILEGE BOUNDARY. Be precise, because a reader who assumed +-- otherwise would trust it for something it does not do: +-- * A custom GUC is settable by any session. Nothing stops a connection that already has DELETE on +-- these tables from issuing the same SET LOCAL itself and then deleting from them directly. The +-- marker decides whether a *permitted* delete is refused; it is not what decides permission. +-- * The guards still do not bind a role that can ALTER TABLE -- which is the application role, because +-- it created the tables (0006:46-62). An owner can disable or drop a trigger and write what it likes. +-- * Conversely, the EXECUTE grant above is a real privilege boundary, and the only one here: a role +-- without it cannot reach the purge at all, whatever it does with the GUC. +-- What this buys is narrower and real: erasure has exactly ONE code path, inside ONE transaction, with the +-- guard never switched off, never left off across a failure, and never widened for any other session. It +-- is a guard against a bug, a careless script, or a compromised application path -- not against an +-- administrator who has decided to tamper. A deployment that needs more must own these tables with a role +-- the application does not have. +-- +-- WHAT DELETION DOES NOT REACH. Backups, replicas, WAL and logical-replication streams, exported +-- telemetry, and any external artifact a record merely named are host-owned and out of reach of this +-- schema. +-- +-- AND ONE THING INSIDE THIS DATABASE: THE DEAD TUPLE. Step 9 is an UPDATE, and an UPDATE in PostgreSQL +-- writes a new row version and leaves the old one in the heap. Until VACUUM reclaims it, the previous +-- version of the record row is still on disk and STILL CARRIES THE ERASED TEXT -- the task summary, the +-- lesson, the attempt results, the task id -- readable by anyone who can inspect the page (pageinspect, +-- a file-level copy, a base backup taken in that window). The same is true of every DELETE above. State +-- this plainly rather than reassuringly: the *index* entries that go dead alongside them point at rows +-- that no longer carry the erased text and so cannot return it, but the *heap* tuple they point at is +-- the erased text, for as long as it survives. An erasure obligation with a deadline has to reach it: +-- +-- VACUUM (VERBOSE) agent_experience.experience_records; -- and the tables swept above +-- +-- Ordinary VACUUM reclaims a dead tuple once no snapshot can still see it; autovacuum will get there on +-- its own schedule, which is not a schedule anybody promised. VACUUM does not overwrite the freed bytes, +-- so a deployment that must also defeat forensic recovery of freed pages needs VACUUM FULL (which +-- rewrites the table under an ACCESS EXCLUSIVE lock) or a storage-level guarantee, neither of which this +-- script can give it. +-- +-- THE THREE INDEXES ARE NOT FREE ON A LARGE, HAND-APPLIED DATABASE, AND ONE OF THEM IS OVER THE BIGGEST +-- TABLE HERE. ix_experience_records_live_by_age, ix_confidence_evidence_experience and +-- ix_reuse_feedback_exposures_experience are built with plain CREATE INDEX inside the migrator's +-- per-script transaction, which takes a SHARE lock and therefore blocks every write to those tables for +-- the duration of the build. On a fresh database that is imperceptible; on an established one with a long +-- record history it is a write outage, and it is a larger one than 0007's, 0008's or 0009's, because +-- experience_records is the table this library writes most. A deployment that cannot take one should +-- create all three out of band *before* running this script -- CREATE INDEX ... CONCURRENTLY cannot run +-- inside a transaction block at all, and IF NOT EXISTS then makes this script's own statements no-ops: +-- +-- CREATE INDEX CONCURRENTLY IF NOT EXISTS ix_experience_records_live_by_age +-- ON agent_experience.experience_records +-- (tenant_id, application_id, project_id, created_at, experience_id) +-- WHERE deleted_at IS NULL; +-- CREATE INDEX CONCURRENTLY IF NOT EXISTS ix_confidence_evidence_experience +-- ON agent_experience.confidence_evidence (experience_id); +-- CREATE INDEX CONCURRENTLY IF NOT EXISTS ix_reuse_feedback_exposures_experience +-- ON agent_experience.reuse_feedback_exposures (experience_id); +-- +-- The first of those needs the deleted_at column, so add it first and separately -- ADD COLUMN ... NULL +-- rewrites nothing and takes only a brief ACCESS EXCLUSIVE lock: +-- +-- ALTER TABLE agent_experience.experience_records ADD COLUMN IF NOT EXISTS deleted_at timestamptz NULL; +-- +-- CONCURRENTLY can leave an INVALID index behind if it fails; check with +-- "SELECT indisvalid FROM pg_index WHERE indexrelid = 'agent_experience.ix_experience_records_live_by_age'::regclass", +-- and DROP INDEX CONCURRENTLY and retry if it comes back false. Do this before migrating, not after. +-- +-- UPGRADING AN EXISTING DATABASE. The new column is nullable, so no row is rewritten and no default is +-- backfilled. The tombstone-shape CHECK is ADD CONSTRAINT ... NOT VALID exactly as 0006's and 0007's are: +-- every existing row has deleted_at IS NULL and therefore satisfies it, so validation would in fact +-- succeed -- but a scan of a large record table at startup is a cost no deployment asked for. After +-- upgrading, confirm and then validate at a time of your choosing: +-- +-- SELECT experience_id FROM agent_experience.experience_records +-- WHERE (deleted_at IS NULL) <> (status <> 'Deleted') +-- OR (deleted_at IS NOT NULL AND (payload <> '{}'::jsonb OR task_id <> '(deleted)')); +-- +-- Once it returns nothing: +-- +-- ALTER TABLE agent_experience.experience_records VALIDATE CONSTRAINT experience_records_tombstone_shape; +-- +-- VALIDATE takes only a SHARE UPDATE EXCLUSIVE lock, so it does not block reads or writes. + +ALTER TABLE agent_experience.experience_records + ADD COLUMN IF NOT EXISTS deleted_at timestamptz NULL; + +DO $body$ +BEGIN + -- A tombstone is one shape or it is not a tombstone. Without this, a writer could set deleted_at on a + -- row that still holds its payload -- a record that reads as erased everywhere while the text it was + -- deleted for is still sitting in the table. + -- + -- WHAT THIS CHECK DOES NOT PIN, SAID HERE SO NOBODY READS MORE INTO IT. It constrains an existing + -- row's shape, not its history. A row can be INSERTed as a tombstone directly, with any created_at, + -- updated_at or deleted_at the writer likes -- there is no UPDATE for enforce_record_projection to + -- refuse, and a tombstone's timestamps are checked against each other only in that trigger, on the + -- one transition that creates one. So "a tombstone's only timestamp is the moment it was erased" is + -- a property of agent_experience.purge_experience_record, and of every tombstone this library made; + -- it is not a property the schema can prove about a row somebody else inserted. The same goes for + -- the revision on such a row: only the purge's UPDATE is made to advance it by exactly one. + IF NOT EXISTS ( + SELECT 1 FROM pg_constraint + WHERE conname = 'experience_records_tombstone_shape' + AND conrelid = 'agent_experience.experience_records'::regclass) + THEN + ALTER TABLE agent_experience.experience_records + ADD CONSTRAINT experience_records_tombstone_shape + CHECK ( + (deleted_at IS NULL AND status <> 'Deleted') + OR (deleted_at IS NOT NULL + AND status = 'Deleted' + AND task_id = '(deleted)' + AND payload = '{}'::jsonb + AND source_run_id = '00000000-0000-0000-0000-000000000000'::uuid + AND reuse_confidence = 0 + AND supporting_validations = 0 + AND contradictions = 0)) + NOT VALID; + END IF; +END +$body$; + +-- The retention sweep's whole predicate: a scope's live records, oldest first. Partial on +-- deleted_at IS NULL, so the index holds only rows a sweep could still act on and tombstones cost nothing +-- to keep. It also serves every read's "and not a tombstone" filter for a scope that has accumulated them. +CREATE INDEX IF NOT EXISTS ix_experience_records_live_by_age + ON agent_experience.experience_records (tenant_id, application_id, project_id, created_at, experience_id) + WHERE deleted_at IS NULL; + +-- 0007 said this index "belongs with the query that justifies it, not ahead of it", and named this story. +-- Here is the query: the erasure sweeps the evidence ledger by the record it is about, and that record has +-- no other handle on this table -- confidence_evidence carries no scope columns and no foreign key. +CREATE INDEX IF NOT EXISTS ix_confidence_evidence_experience + ON agent_experience.confidence_evidence (experience_id); + +-- The same, for the exposure ledger: its primary key is (feedback_id, experience_id), so deleting by the +-- record alone would scan it. 0008 deferred this index to this story for the same reason 0007 did. +CREATE INDEX IF NOT EXISTS ix_reuse_feedback_exposures_experience + ON agent_experience.reuse_feedback_exposures (experience_id); + +-- The marker, read in one place so no guard retypes the parameter name or the default. A custom GUC that +-- was never set reads back NULL under the missing_ok form, which is exactly "not authorized". +CREATE OR REPLACE FUNCTION agent_experience.purge_authorized() RETURNS boolean +LANGUAGE sql +STABLE +SET search_path = pg_catalog +AS $body$ + SELECT coalesce(pg_catalog.current_setting('agent_experience.purge_authorized', true), 'off') = 'on'; +$body$; + +-- 0006's append-only guard, extended with exactly one exception and no others. UPDATE and TRUNCATE are +-- still refused unconditionally, in every session including the purging one: erasure removes rows, it +-- never rewrites them, and a TRUNCATE is never scoped to one record. DELETE is admitted only while the +-- marker is set and only on the five tables an erasure sweeps -- experience_grant_access is deliberately +-- absent, because who read a record before it was deleted outlives the record. +-- +-- The refusal message and SQLSTATE are unchanged, so an operator who hits the guard is told exactly what +-- 0006 told them. +CREATE OR REPLACE FUNCTION agent_experience.reject_event_log_mutation() RETURNS trigger AS $body$ +BEGIN + IF TG_OP = 'DELETE' + AND agent_experience.purge_authorized() + AND TG_TABLE_NAME IN ( + 'lifecycle_events', + 'experience_grant_events', + 'confidence_evidence', + 'reuse_feedback', + 'reuse_feedback_exposures') + THEN + RETURN OLD; + END IF; + + RAISE EXCEPTION + 'agent_experience.% is append-only: a stored event row cannot be %.', + TG_TABLE_NAME, + CASE TG_OP + WHEN 'UPDATE' THEN 'updated' + WHEN 'DELETE' THEN 'deleted' + ELSE 'truncated away' + END + USING ERRCODE = 'insufficient_privilege'; +END; +$body$ LANGUAGE plpgsql; + +-- THE RECORD ROW ITSELF CANNOT BE REMOVED, BY ANYBODY, MARKER OR NOT. Until this script there was no +-- guard here at all: a bare +-- +-- DELETE FROM agent_experience.experience_records WHERE experience_id = ...; +-- +-- succeeded from any session with DELETE on the table, and it is the one statement that undoes everything +-- the erasure above is for. It orphans the whole audit trail -- lifecycle_events, confidence_evidence and +-- the exposure ledger have no foreign key to experience_records, deliberately (0002, 0007, 0008), so +-- their rows simply outlive the record and name an ID that no longer resolves -- and, worse, it FREES THE +-- ID: experience_grants has no foreign key either (0005), so re-inserting a record under the same +-- experience_id makes every grant that was issued over the old content apply to the new content. That is +-- precisely what step 6 of the erasure exists to prevent, and it was reachable by exactly the actor this +-- script's honesty statement names as in scope -- a bug, a careless script, or a compromised application +-- path. +-- +-- There is no exception and no marker clause, because the erasure never deletes this row: it UPDATEs it +-- into a tombstone (step 9), and the tombstone is the point. So "no supported path removes a record" is +-- now enforced by the schema rather than promised by the documentation. The escape hatch is the same one +-- every other guard here has and no smaller: the table's owner can ALTER TABLE ... DISABLE TRIGGER, which +-- is a deliberate, visible act by a role that could drop the table anyway. +-- +-- TRUNCATE is refused for the same reason and with the same finality. +CREATE OR REPLACE FUNCTION agent_experience.reject_record_removal() RETURNS trigger AS $body$ +BEGIN + RAISE EXCEPTION + 'An Experience Record row is never removed: erasure leaves a payload-free tombstone under the same ' + 'experience_id, through agent_experience.purge_experience_record, so the ID can never be reused.' + USING ERRCODE = 'insufficient_privilege'; +END; +$body$ LANGUAGE plpgsql; + +-- CREATE TRIGGER has no IF NOT EXISTS, and dropping one to recreate it would leave a window in which the +-- table is unguarded, so each is created only when it is absent -- exactly as 0006 does. ENABLE ALWAYS is +-- applied unconditionally afterwards: a no-op on a trigger that already has it, and what makes the guard +-- survive session_replication_role = 'replica'. +DO $body$ +BEGIN + IF NOT EXISTS ( + SELECT 1 FROM pg_trigger + WHERE tgname = 'experience_records_no_delete' + AND tgrelid = 'agent_experience.experience_records'::regclass) + THEN + CREATE TRIGGER experience_records_no_delete + BEFORE DELETE ON agent_experience.experience_records + FOR EACH ROW EXECUTE FUNCTION agent_experience.reject_record_removal(); + END IF; + + IF NOT EXISTS ( + SELECT 1 FROM pg_trigger + WHERE tgname = 'experience_records_no_truncate' + AND tgrelid = 'agent_experience.experience_records'::regclass) + THEN + CREATE TRIGGER experience_records_no_truncate + BEFORE TRUNCATE ON agent_experience.experience_records + FOR EACH STATEMENT EXECUTE FUNCTION agent_experience.reject_record_removal(); + END IF; +END +$body$; + +ALTER TABLE agent_experience.experience_records ENABLE ALWAYS TRIGGER experience_records_no_delete; +ALTER TABLE agent_experience.experience_records ENABLE ALWAYS TRIGGER experience_records_no_truncate; + +-- 0006's grant-delete guard, with the same one exception. The audit-trail rule is unchanged for every +-- ordinary writer: a grant that has events cannot be deleted, because the row would go and the trail would +-- stay. Inside a purge the trail has already gone -- the erasure order deletes experience_grant_events +-- first, in the same transaction -- so the EXISTS below would pass anyway; the marker is tested explicitly +-- so that the rule reads as one deliberate exception rather than as a coincidence of ordering. TRUNCATE +-- stays refused unconditionally. +CREATE OR REPLACE FUNCTION agent_experience.reject_audited_grant_delete() RETURNS trigger AS $body$ +BEGIN + IF TG_OP = 'TRUNCATE' THEN + RAISE EXCEPTION + 'agent_experience.experience_grants cannot be truncated: its audit trail would outlive it.' + USING ERRCODE = 'insufficient_privilege'; + END IF; + + IF agent_experience.purge_authorized() THEN + RETURN OLD; + END IF; + + IF EXISTS (SELECT 1 FROM agent_experience.experience_grant_events e WHERE e.grant_id = OLD.grant_id) THEN + RAISE EXCEPTION + 'A grant with an audit trail cannot be deleted: revoke it instead, so the trail and the row agree.' + USING ERRCODE = 'insufficient_privilege'; + END IF; + + RETURN OLD; +END; +$body$ LANGUAGE plpgsql; + +-- The projection guard, extended twice: once to make a tombstone terminal, and once to admit the one +-- UPDATE that creates it. +-- +-- WHY THE GUARD HAD TO CHANGE AT ALL. 0007's rule says reuse_confidence and its counters may only move to +-- values a lifecycle event recorded for the new revision. A tombstone zeroes all three, and the erasure +-- has just removed every lifecycle event the record had, so there is no event to point at and the guard +-- would reject the very statement the purge exists to perform. Rather than leave the numbers behind -- +-- they are a summary of how often this record's lesson held up, which is exactly what a deletion is asked +-- to remove -- the guard recognises the tombstone shape under the same marker the append-only guards read. +-- +-- THE EXCEPTION IS SHAPE-CHECKED, NOT MERELY MARKER-CHECKED, AND THE SHAPE INCLUDES THE SCOPE. A marked +-- transaction may make this one transition and no other: a live row, to a row whose deleted_at is set, +-- whose payload is empty, whose task_id is the placeholder, whose status is the tombstone literal, whose +-- created_at and updated_at are the deletion instant, whose payload_version and six scope columns are +-- unchanged, one revision forward. Checking only the payload columns would have left a marked UPDATE free +-- to move the row's scope while erasing it -- tombstoning a record INTO ANOTHER TENANT'S SCOPE, so that +-- the scope that owned it sees NotFound for its own erased record and a scope that never held it sees +-- Deleted. The scope equality below is what makes the tombstone answerable to, and only to, the scope +-- that owned the record. +-- +-- A marked UPDATE that sets deleted_at and does NOT match the shape is refused outright rather than +-- falling through to the ordinary rules: the ordinary rules are about live projections, and on a row +-- whose counters already read 0/0/0 they would have let a malformed tombstone through. +-- +-- A TOMBSTONE IS TERMINAL. No UPDATE of a row whose deleted_at is already set is admitted, marker or not. +-- That is what makes "deleting twice touches nothing" and "a late lifecycle commit cannot move a +-- tombstone" true of the schema rather than only of the adapter. And outside a purge, deleted_at cannot be +-- set, cleared, or changed at all: erasure has one code path. +CREATE OR REPLACE FUNCTION agent_experience.enforce_record_projection() RETURNS trigger AS $body$ +BEGIN + IF NEW.experience_id IS DISTINCT FROM OLD.experience_id THEN + RAISE EXCEPTION + 'An Experience Record''s identity is fixed; its lifecycle events name it.' + USING ERRCODE = 'insufficient_privilege'; + END IF; + + IF NEW.revision < OLD.revision THEN + RAISE EXCEPTION + 'An Experience Record''s revision only moves forward: % cannot follow %.', NEW.revision, OLD.revision + USING ERRCODE = 'insufficient_privilege'; + END IF; + + IF OLD.deleted_at IS NOT NULL THEN + RAISE EXCEPTION + 'An erased Experience Record is a tombstone: it carries no payload and cannot be changed again.' + USING ERRCODE = 'insufficient_privilege'; + END IF; + + IF NEW.deleted_at IS NOT NULL THEN + IF NOT agent_experience.purge_authorized() THEN + RAISE EXCEPTION + 'An Experience Record is erased only through agent_experience.purge_experience_record.' + USING ERRCODE = 'insufficient_privilege'; + END IF; + + -- The authorized exception: exactly the tombstone, and only while the marker is set. Anything + -- else that sets deleted_at stops here rather than continuing into the live-projection rules. + IF NEW.revision = OLD.revision + 1 + AND NEW.status = 'Deleted' + AND NEW.task_id = '(deleted)' + AND NEW.payload = '{}'::jsonb + AND NEW.payload_version = OLD.payload_version + AND NEW.created_at = NEW.deleted_at + AND NEW.updated_at = NEW.deleted_at + AND NEW.tenant_id = OLD.tenant_id + AND NEW.application_id = OLD.application_id + AND NEW.project_id = OLD.project_id + AND NEW.team_id IS NOT DISTINCT FROM OLD.team_id + AND NEW.agent_id IS NOT DISTINCT FROM OLD.agent_id + AND NEW.user_id IS NOT DISTINCT FROM OLD.user_id + THEN + RETURN NEW; + END IF; + + RAISE EXCEPTION + 'A purge may make exactly one transition and no other: a live Experience Record into its own ' + 'tombstone, in its own scope, one revision forward.' + USING ERRCODE = 'insufficient_privilege'; + END IF; + + IF NEW.status IS DISTINCT FROM OLD.status AND NEW.revision <= OLD.revision THEN + RAISE EXCEPTION + 'An Experience Record''s status changes only with the revision its lifecycle event produced.' + USING ERRCODE = 'insufficient_privilege'; + END IF; + + IF NEW.reuse_confidence IS DISTINCT FROM OLD.reuse_confidence + OR NEW.supporting_validations IS DISTINCT FROM OLD.supporting_validations + OR NEW.contradictions IS DISTINCT FROM OLD.contradictions + THEN + IF NEW.revision <= OLD.revision THEN + RAISE EXCEPTION + 'An Experience Record''s reuse confidence and evidence counters change only with the revision ' + 'of the lifecycle event that recorded the evidence for them.' + USING ERRCODE = 'insufficient_privilege'; + END IF; + + IF NOT EXISTS ( + SELECT 1 FROM agent_experience.lifecycle_events e + WHERE e.experience_id = NEW.experience_id + AND e.applied_revision = NEW.revision + AND e.confidence_evidence_id IS NOT NULL + AND e.new_reuse_confidence = NEW.reuse_confidence + AND e.new_supporting_validations = NEW.supporting_validations + AND e.new_contradictions = NEW.contradictions) + THEN + RAISE EXCEPTION + 'An Experience Record''s reuse confidence and evidence counters may only be set to the values ' + 'a lifecycle event recorded for revision %.', NEW.revision + USING ERRCODE = 'insufficient_privilege'; + END IF; + END IF; + + RETURN NEW; +END; +$body$ LANGUAGE plpgsql; + +-- THE ONE ERASURE PATH. One transaction, one order, every step inside this function. +-- +-- The order is not incidental and must not be rearranged: +-- 1. experience_records SELECT ... FOR UPDATE with the scope and revision guards. Establishes +-- authorization and pins the row. A row that does not match is disambiguated by +-- a second, scope-only read inside this same transaction, exactly as the store's +-- lifecycle commit disambiguates its own "no row updated". +-- 2. confidence_evidence MUST precede step 9. It has no scope columns and no foreign key (0007), so the +-- record row's scope is the only thing that makes it reachable by scope at all. +-- 3. reuse_feedback_exposures The exposures naming this record. Children before parents: the foreign key +-- to reuse_feedback is NO ACTION. The submissions those exposures belong to are +-- locked FOR UPDATE *first*, see below. +-- 4. reuse_feedback Only the submissions step 3 emptied. A submission that also named other records +-- keeps its row -- it still describes them -- and only its exposure of this +-- record is gone. +-- 5. experience_grant_events Before the grants themselves: reject_audited_grant_delete refuses to delete +-- a grant that still has events. +-- 6. experience_grants Legal only once step 5 emptied the trail. Grants are purged with the record +-- because experience_grants has no foreign key to it (0005) and a re-appearing ID +-- would otherwise re-apply them. +-- 7. lifecycle_events The record's own history. +-- 8. experience_embeddings Guarded by to_regclass and a column check, and run through EXECUTE, because +-- the table belongs to the vectors package (0004) and this package must not +-- depend on it. A base-only deployment simply skips the step. (0004's foreign key +-- is ON DELETE CASCADE, but nothing here deletes the record row -- and nothing +-- can, see reject_record_removal -- so the row must be removed explicitly.) +-- +-- NOTE WHAT STEPS 2-8 DO NOT CARRY: A SCOPE PREDICATE. Every one of them matches on the record's ID alone. +-- For steps 2-4 that is forced -- confidence_evidence and the feedback ledger have no scope columns at all +-- (0007, 0008) -- and for steps 5-8 it is a choice, stated here rather than left to be discovered, because +-- disclosing it for some of the steps and not the others would read as though the others were scoped: +-- * experience_grants and experience_grant_events DO have the six owner-scope columns, and every grant +-- this library issues copies them from the record row (IssueGrantSql), so in practice the scope +-- predicate would match the same rows. A grant written outside this library with a different owner +-- scope over this record's ID is deleted anyway, and deliberately: "every row that named this record" +-- is what erasure means here, and a grant over an ID whose content is gone is exactly the row nothing +-- else would collect. The same reasoning covers a feedback exposure another scope recorded against +-- this ID, which 0008 deliberately allows. +-- * experience_embeddings likewise carries copied scope columns and is matched by ID for the same reason. +-- Authorization is decided ONCE, at step 1, over the record itself: a caller who cannot pass the scope and +-- revision guards there never reaches step 2. What follows is not a second authorization check and must +-- not be read as one. +-- 9. experience_records The tombstone, last, so every scope-dependent sweep above still had its scope. +-- +-- SECURITY DEFINER is what makes the marker meaningful as a single code path rather than as a privilege: +-- see the honesty statement in this script's header. search_path is pinned so nothing here resolves +-- through a caller's. +CREATE OR REPLACE FUNCTION agent_experience.purge_experience_record( + p_experience_id uuid, + p_tenant_id text, + p_application_id text, + p_project_id text, + p_team_id text, + p_agent_id text, + p_user_id text, + p_expected_revision bigint, + p_deleted_at timestamptz) +RETURNS TABLE (purge_outcome text, purge_revision bigint) +LANGUAGE plpgsql +SECURITY DEFINER +SET search_path = pg_catalog, agent_experience +SET agent_experience.purge_authorized = 'off' +AS $body$ +DECLARE + v_revision bigint; + v_deleted_at timestamptz; + v_feedback_ids uuid[]; +BEGIN + -- Transaction-scoped, and narrower still: the function declares a SET for the same variable, so the + -- marker is restored at function exit even when the caller's transaction continues afterwards. + SET LOCAL agent_experience.purge_authorized = 'on'; + + -- Step 1. The scope predicate, the revision guard, and the existence check in one statement, so a + -- foreign scope, a stale revision, and a missing record are all "no row" and none of them can be told + -- apart from the outcome, from a timing branch, or from the error text. + SELECT r.revision, r.deleted_at INTO v_revision, v_deleted_at + FROM agent_experience.experience_records r + WHERE r.experience_id = p_experience_id + AND r.tenant_id = p_tenant_id + AND r.application_id = p_application_id + AND r.project_id = p_project_id + AND r.team_id IS NOT DISTINCT FROM p_team_id + AND r.agent_id IS NOT DISTINCT FROM p_agent_id + AND r.user_id IS NOT DISTINCT FROM p_user_id + AND r.deleted_at IS NULL + AND (p_expected_revision IS NULL OR r.revision = p_expected_revision) + FOR UPDATE; + + IF NOT FOUND THEN + -- Re-read inside the same transaction, with the same scope predicate and without the revision and + -- tombstone guards, so "not in this scope" stays indistinguishable from "does not exist" while a + -- stale revision and an already-erased record can still be reported to the scope that owns them. + SELECT r.revision, r.deleted_at INTO v_revision, v_deleted_at + FROM agent_experience.experience_records r + WHERE r.experience_id = p_experience_id + AND r.tenant_id = p_tenant_id + AND r.application_id = p_application_id + AND r.project_id = p_project_id + AND r.team_id IS NOT DISTINCT FROM p_team_id + AND r.agent_id IS NOT DISTINCT FROM p_agent_id + AND r.user_id IS NOT DISTINCT FROM p_user_id + FOR UPDATE; + + IF NOT FOUND THEN + RETURN QUERY SELECT 'NotFound'::text, 0::bigint; + RETURN; + END IF; + + IF v_deleted_at IS NOT NULL THEN + -- Already a tombstone. Deleting again is a success that touches nothing. + RETURN QUERY SELECT 'AlreadyDeleted'::text, v_revision; + RETURN; + END IF; + + RETURN QUERY SELECT 'StaleRevision'::text, v_revision; + RETURN; + END IF; + + -- Step 2. + DELETE FROM agent_experience.confidence_evidence WHERE experience_id = p_experience_id; + + -- Step 3, in three parts, and the order of them is the whole defence against two purges sharing one + -- submission. + -- + -- THE RACE THIS AVOIDS. A submission may name several records; two purges erasing two of them run + -- concurrently. If each simply deleted its own exposure and then asked "does this submission have any + -- exposures left?", each would still see the other's not-yet-committed exposure row -- READ COMMITTED + -- hides an uncommitted delete -- so each would decide the submission is still describing something and + -- leave it. Both commit; the submission survives with ZERO exposures, describing nothing, and nothing + -- else ever collects it. It carries a run ID, a scope, an outcome, a measure and -- for a human + -- assessment -- a reviewer identity and a free-text rationale about records that no longer exist. + -- + -- So: find the submissions this record's exposures belong to, take a row lock on each of them in a + -- deterministic order, and only then delete. The second purge blocks on that lock until the first has + -- committed, and its "any exposures left?" then runs against a snapshot that can see the first's + -- delete. Locking the parents (rather than re-checking afterwards) also means the two purges cannot + -- interleave into a deadlock: the order is by feedback_id for both. + SELECT array_agg(DISTINCT x.feedback_id) INTO v_feedback_ids + FROM agent_experience.reuse_feedback_exposures x + WHERE x.experience_id = p_experience_id; + + IF v_feedback_ids IS NOT NULL THEN + PERFORM 1 FROM agent_experience.reuse_feedback f + WHERE f.feedback_id = ANY(v_feedback_ids) + ORDER BY f.feedback_id + FOR UPDATE; + END IF; + + DELETE FROM agent_experience.reuse_feedback_exposures x WHERE x.experience_id = p_experience_id; + + -- Step 4. Only the submissions step 3 emptied -- "every submission with no exposures" would be a + -- different, and much larger, statement. + IF v_feedback_ids IS NOT NULL THEN + DELETE FROM agent_experience.reuse_feedback f + WHERE f.feedback_id = ANY(v_feedback_ids) + AND NOT EXISTS ( + SELECT 1 FROM agent_experience.reuse_feedback_exposures x WHERE x.feedback_id = f.feedback_id); + END IF; + + -- Step 5. + DELETE FROM agent_experience.experience_grant_events + WHERE grant_id IN ( + SELECT g.grant_id FROM agent_experience.experience_grants g WHERE g.experience_id = p_experience_id); + + -- Step 6. + DELETE FROM agent_experience.experience_grants WHERE experience_id = p_experience_id; + + -- Step 7. + DELETE FROM agent_experience.lifecycle_events WHERE experience_id = p_experience_id; + + -- Step 8. to_regclass answers "is there a relation by that name", which is not the same question as + -- "is it the vectors package's embedding table". A deployment that has something else under that name + -- -- an old shape, a view, another project's table -- would otherwise fail the EXECUTE with a bare + -- undefined_column and abort the whole erasure, with the record still carrying its payload and no + -- indication of why. So the shape is checked too, and a mismatch is reported as itself. + IF to_regclass('agent_experience.experience_embeddings') IS NOT NULL THEN + IF NOT EXISTS ( + SELECT 1 FROM pg_catalog.pg_attribute a + WHERE a.attrelid = to_regclass('agent_experience.experience_embeddings') + AND a.attname = 'experience_id' + AND a.atttypid = 'pg_catalog.uuid'::pg_catalog.regtype + AND a.attnum > 0 + AND NOT a.attisdropped) + THEN + RAISE EXCEPTION + 'The relation % has no uuid experience_id column, so this record''s stored vector cannot ' + 'be removed. Nothing has been erased; reconcile it with 0004 and retry.', + to_regclass('agent_experience.experience_embeddings') + USING ERRCODE = 'undefined_column'; + END IF; + + EXECUTE 'DELETE FROM agent_experience.experience_embeddings WHERE experience_id = $1' + USING p_experience_id; + END IF; + + -- Step 9. Everything not on the retained list is set to a fixed, content-free value rather than left + -- as it stands: a tombstone must not say when the work happened, which run produced it, or how often + -- its lesson held up. + UPDATE agent_experience.experience_records r + SET payload = '{}'::jsonb, + task_id = '(deleted)', + status = 'Deleted', + source_run_id = '00000000-0000-0000-0000-000000000000'::uuid, + reuse_confidence = 0, + supporting_validations = 0, + contradictions = 0, + created_at = p_deleted_at, + updated_at = p_deleted_at, + deleted_at = p_deleted_at, + revision = r.revision + 1 + WHERE r.experience_id = p_experience_id; + + RETURN QUERY SELECT 'Deleted'::text, v_revision + 1; +END +$body$; + +-- EXPIRED GRANTS. A grant that has expired permits nothing and its row and trail are the only place its +-- recipient scope, its reason, and the administrator who issued it are still written down. This purges +-- them in bounded batches, oldest expiry first, through the same marker and the same erasure ordering +-- rule: the events before the grant they belong to. +-- +-- It also reaches a grant naming a record that is already a tombstone. The record purge removes such +-- grants in its own transaction, and every write path in this library refuses to issue one over a +-- tombstone, so one should not exist -- but "should not" is adapter-enforced, not schema-enforced (there +-- is no foreign key from experience_grants to experience_records, by design in 0005), and a grant naming +-- an erased record is exactly the row nothing else would ever collect. +-- +-- THE BATCH BOUND IS APPLIED HERE, NOT ONLY BY THE CALLER. LIMIT NULL means "no limit" in PostgreSQL, so +-- a hand-caller passing p_limit => NULL would have got an unbounded destructive sweep from a function +-- whose whole contract is that it is bounded. p_limit is clamped below to the same 1..500 the adapter's +-- validator enforces, so the bound is a property of the function rather than of the one caller that +-- happens to go through C#. +-- +-- WHICH CLOCK DECIDES. p_now is the host's, and everywhere a grant is *read* this schema deliberately +-- uses clock_timestamp() instead, "so a caller whose clock is wrong cannot widen anything". Issuing with +-- a wrong clock only narrows a window; deleting with one destroys rows the database still considers live, +-- which a host skewed a day forward would do silently. The cutoff below is therefore +-- LEAST(p_now, clock_timestamp()): the host can make a purge collect less than the database would, never +-- more. +-- +-- A REVOKED-BUT-UNEXPIRED GRANT IS LEFT ALONE. Its revocation is a fact about a window that has not closed +-- yet, and 0006 makes that revocation permanent on purpose; it is collected once it expires like any other. +CREATE OR REPLACE FUNCTION agent_experience.purge_expired_grants( + p_tenant_id text, + p_application_id text, + p_project_id text, + p_team_id text, + p_agent_id text, + p_user_id text, + p_now timestamptz, + p_limit integer) +RETURNS bigint +LANGUAGE plpgsql +SECURITY DEFINER +SET search_path = pg_catalog, agent_experience +SET agent_experience.purge_authorized = 'off' +AS $body$ +DECLARE + v_grant_ids uuid[]; + v_purged bigint; + -- 1 and 500 are the same bounds ExperienceRecordValidator enforces on the adapter's batchSize. A NULL + -- becomes the maximum rather than an error, because a bound is what this function owes its caller and + -- refusing would tell a hand-caller nothing it could not work out from the signature. + v_limit integer := least(greatest(coalesce(p_limit, 500), 1), 500); + v_cutoff timestamptz := least(p_now, pg_catalog.clock_timestamp()); +BEGIN + SET LOCAL agent_experience.purge_authorized = 'on'; + + SELECT array_agg(expired.grant_id) INTO v_grant_ids + FROM ( + SELECT g.grant_id + FROM agent_experience.experience_grants g + WHERE g.tenant_id = p_tenant_id + AND g.application_id = p_application_id + AND g.project_id = p_project_id + AND g.team_id IS NOT DISTINCT FROM p_team_id + AND g.agent_id IS NOT DISTINCT FROM p_agent_id + AND g.user_id IS NOT DISTINCT FROM p_user_id + AND (g.expires_at <= v_cutoff + OR EXISTS ( + SELECT 1 FROM agent_experience.experience_records r + WHERE r.experience_id = g.experience_id AND r.deleted_at IS NOT NULL)) + ORDER BY g.expires_at, g.grant_id + LIMIT v_limit + FOR UPDATE) expired; + + IF v_grant_ids IS NULL THEN + RETURN 0::bigint; + END IF; + + DELETE FROM agent_experience.experience_grant_events WHERE grant_id = ANY(v_grant_ids); + + WITH removed AS ( + DELETE FROM agent_experience.experience_grants WHERE grant_id = ANY(v_grant_ids) RETURNING 1) + SELECT count(*) INTO v_purged FROM removed; + + RETURN v_purged; +END +$body$; + +-- WHO MAY ERASE. Both functions above are SECURITY DEFINER, and PostgreSQL grants EXECUTE on a function to +-- PUBLIC by default -- which would make them callable by every role that can connect, including one with +-- no DELETE or UPDATE privilege anywhere in this schema, over any tenant whose experience_id and scope it +-- can SELECT. That is the one genuine privilege escalation this script could introduce, so it is revoked +-- here and granted back only to the role applying the migration, which owns these tables and is the role +-- the application runs as. CREATE OR REPLACE FUNCTION preserves a function's ACL, so re-running this +-- script neither loses the revoke nor re-opens the grant; both statements are idempotent. +-- +-- A deployment whose application role is NOT the migrating role must grant EXECUTE to it explicitly -- see +-- this script's header for the two statements -- and should grant it to nothing else. This is the only +-- privilege boundary in this script; the marker is not one, and never claimed to be. +REVOKE ALL ON FUNCTION agent_experience.purge_experience_record( + uuid, text, text, text, text, text, text, bigint, timestamptz) FROM PUBLIC; + +REVOKE ALL ON FUNCTION agent_experience.purge_expired_grants( + text, text, text, text, text, text, timestamptz, integer) FROM PUBLIC; + +GRANT EXECUTE ON FUNCTION agent_experience.purge_experience_record( + uuid, text, text, text, text, text, text, bigint, timestamptz) TO CURRENT_USER; + +GRANT EXECUTE ON FUNCTION agent_experience.purge_expired_grants( + text, text, text, text, text, text, timestamptz, integer) TO CURRENT_USER; + +-- agent_experience.purge_authorized() is deliberately left callable by PUBLIC: it is a STABLE reader of a +-- custom GUC, it reveals only whether the *calling* session set the marker, and the guards above call it +-- from within every session's own triggers. diff --git a/src/AgentExperience.Storage.Postgres/PostgresExperienceCandidateSource.cs b/src/AgentExperience.Storage.Postgres/PostgresExperienceCandidateSource.cs index 83f854a..978103b 100644 --- a/src/AgentExperience.Storage.Postgres/PostgresExperienceCandidateSource.cs +++ b/src/AgentExperience.Storage.Postgres/PostgresExperienceCandidateSource.cs @@ -81,7 +81,17 @@ public sealed class PostgresExperienceCandidateSource : IExperienceCandidateSour $" FROM {PostgresExperienceRecordStore.Table} r {PostgresExperienceRecordStore.PermittingGrantJoin} WHERE "; private const string SearchFilters = - " AND status = ANY(@statuses) " + + // A tombstone carries no payload, so it is not a candidate: its generated search_vector holds + // only the deletion placeholder, and a row that matched it would come back with nothing in it. + // + // Redundant today, and kept on purpose. The status filter below already excludes a tombstone -- + // its status is a literal no ExperienceStatus member names -- so no query this adapter can build + // distinguishes the two, and no test can either. It is stated here rather than left to be + // rediscovered: it is defence in depth against a future status whose name collides, not the + // thing that makes erased text unfindable. What makes erased text unfindable is that + // search_vector is GENERATED ALWAYS and regenerates from the placeholder alone. + $" AND {PostgresExperienceRecordStore.RecordLivePredicate} " + + "AND status = ANY(@statuses) " + "AND reuse_confidence >= @min_confidence " + $"AND search_vector @@ websearch_to_tsquery('{SearchConfiguration}', @task_text) " + $"ORDER BY {RelevanceColumn} DESC, experience_id LIMIT @limit"; diff --git a/src/AgentExperience.Storage.Postgres/PostgresExperienceGrantStore.cs b/src/AgentExperience.Storage.Postgres/PostgresExperienceGrantStore.cs index a976b8d..657791d 100644 --- a/src/AgentExperience.Storage.Postgres/PostgresExperienceGrantStore.cs +++ b/src/AgentExperience.Storage.Postgres/PostgresExperienceGrantStore.cs @@ -82,6 +82,14 @@ public sealed class PostgresExperienceGrantStore : IExperienceGrantStore "(CASE WHEN @expires_at <= now() + @max_lifetime::interval THEN @expires_at END), NULL, NULL " + $"FROM {PostgresExperienceRecordStore.Table} r " + $"WHERE r.experience_id = @experience_id AND {PostgresExperienceRecordStore.RecordScopePredicate} " + + // An erased record is not a record a grant can name: there is nothing left to share, and a grant + // over a tombstone would be a live permission over an ID the erasure spent. experience_grants has + // no foreign key to experience_records (0005), so nothing parks this statement against a + // concurrent erasure by itself -- without the lock it would decide against a snapshot taken + // before the purge committed and issue a 90-day permission over a record that is already gone. + // See PostgresExperienceRecordStore.RecordKeyShareLock. + $"AND {PostgresExperienceRecordStore.RecordLivePredicate} " + + $"{PostgresExperienceRecordStore.RecordKeyShareLock} " + $"RETURNING {GrantColumns}"; /// @@ -115,6 +123,7 @@ public sealed class PostgresExperienceGrantStore : IExperienceGrantStore $"FROM {PostgresExperienceRecordStore.Table} r " + $"LEFT JOIN {PostgresExperienceRecordStore.GrantsTable} g ON g.experience_id = r.experience_id " + $"WHERE r.experience_id = @experience_id AND {PostgresExperienceRecordStore.RecordScopePredicate} " + + $"AND {PostgresExperienceRecordStore.RecordLivePredicate} " + "ORDER BY g.issued_at, g.grant_id LIMIT @limit"; /// One grant's trail, oldest first, alongside the grant as it stands now. @@ -147,6 +156,15 @@ public sealed class PostgresExperienceGrantStore : IExperienceGrantStore "@reason, @administrator_principal_id, @administrator_authorized_at, g.expires_at, now(), now() " + $"FROM {PostgresExperienceRecordStore.GrantsTable} g WHERE g.grant_id = @grant_id"; + /// + /// The expired-grant purge, created by 0010. Bounded, scoped, and through the same + /// transaction-scoped marker the record erasure uses, so the append-only guard over + /// experience_grant_events is never switched off and never widened for another session. + /// + private const string PurgeExpiredGrantsSql = + "SELECT agent_experience.purge_expired_grants(" + + "@tenant_id, @application_id, @project_id, @team_id, @agent_id, @user_id, @now, @limit)"; + /// The primary key a re-issued violates. private const string GrantPrimaryKey = "experience_grants_pkey"; @@ -459,6 +477,105 @@ await AppendEventAsync(connection, transaction, revoked.GrantId, RevokedAction, } } + /// + /// Removes the grants in one owner scope that have expired, together with their audit events, in one + /// bounded batch. + /// + /// + /// + /// Why an expired grant is deleted rather than kept. It permits nothing -- the read predicate + /// stopped admitting it the moment it expired -- and its row and trail are the only place its + /// recipient scope, its stated reason, and the administrator who issued it are still written down. + /// Keeping them forever is keeping personal data for a permission that no longer exists. What a + /// delivery actually happened under is kept separately and is not touched here: + /// experience_grant_access is retained by design. + /// + /// + /// A revoked grant that has not expired yet is left alone. Its revocation is a fact about a + /// window that is still open, and 0006 makes that revocation permanent on purpose; it is + /// collected once it expires like any other. Grants naming a record that is already a tombstone are + /// collected too -- the record erasure removes them in its own transaction, so one can only survive + /// if it was written outside this library. + /// + /// + /// Like the record sweep, this runs only when a host calls it: there is no timer here, no background + /// service, and no default schedule. + /// + /// + /// What the host has established the caller may do. + /// The host-constructed administrator authority. A purge is a grant mutation and needs one, exactly as issuing and revoking do. + /// The exact owner scope to purge within. Never treated as authority. + /// The most grants this call may remove, from to . + /// Cancels the operation. + /// + /// when the purge ran (possibly removing nothing), + /// , or . + /// + /// or is . + public async Task PurgeExpiredAsync( + AuthorizationContext authorization, + GrantAdministration? administration, + Scope recordScope, + int batchSize, + CancellationToken cancellationToken) + { + ArgumentNullException.ThrowIfNull(authorization); + ArgumentNullException.ThrowIfNull(recordScope); + + var errors = ExperienceRecordValidator.ValidateGrantPurge(recordScope, batchSize); + if (errors.Count > 0) + { + return new(ExperienceStoreOutcome.Invalid, 0, false, errors); + } + + if (Administrator(administration) is null) + { + return new(ExperienceStoreOutcome.Denied, 0, false, NoErrors); + } + + if (!authorization.Permits(recordScope)) + { + return new(ExperienceStoreOutcome.Denied, 0, false, NoErrors); + } + + var administrationErrors = ExperienceRecordValidator.ValidateAdministration(administration!); + if (administrationErrors.Count > 0) + { + return new(ExperienceStoreOutcome.Invalid, 0, false, administrationErrors); + } + + cancellationToken.ThrowIfCancellationRequested(); + + try + { + await using var connection = await _dataSource.OpenConnectionAsync(cancellationToken).ConfigureAwait(false); + await using var command = new NpgsqlCommand(PurgeExpiredGrantsSql, connection); + var parameters = command.Parameters; + PostgresExperienceRecordStore.AddScopeParameters(parameters, recordScope); + + // This store's own clock decides which grants are past their expiry, the same way it decides + // the maximum lifetime a new grant may be issued with. Whether a grant still *permits* a read + // is always the database's clock_timestamp(), which no host can wind. + parameters.Add(new NpgsqlParameter( + "now", + PostgresExperienceRecordStore.ToStoredTimestamp(_timeProvider.GetUtcNow()))); + parameters.Add(new NpgsqlParameter("limit", batchSize)); + + var purged = await command.ExecuteScalarAsync(cancellationToken).ConfigureAwait(false) is long count + ? (int)count + : 0; + + // A full batch is the only evidence this call has that more may be waiting: the purge + // function reports what it removed, and asking a second question would answer about a + // different moment. + return new(ExperienceStoreOutcome.Deleted, purged, purged >= batchSize, NoErrors); + } + catch (Exception ex) when (PostgresExperienceRecordStore.IsInfrastructureFailure(ex, cancellationToken)) + { + throw PostgresExperienceRecordStore.Translate(ex, "grant purge", cancellationToken); + } + } + /// public async Task ListAsync( AuthorizationContext authorization, diff --git a/src/AgentExperience.Storage.Postgres/PostgresExperienceRecordSchema.cs b/src/AgentExperience.Storage.Postgres/PostgresExperienceRecordSchema.cs index 1b000b5..6b2e424 100644 --- a/src/AgentExperience.Storage.Postgres/PostgresExperienceRecordSchema.cs +++ b/src/AgentExperience.Storage.Postgres/PostgresExperienceRecordSchema.cs @@ -99,6 +99,41 @@ public static class PostgresExperienceRecordSchema /// public const string GrantAccessLogScriptName = "0009_grant_access_log.sql"; + /// + /// The script that adds experience_records.deleted_at and the one erasure path: + /// agent_experience.purge_experience_record, which removes every payload-bearing row that + /// names one record and leaves a payload-free tombstone behind, plus + /// agent_experience.purge_expired_grants for grants that have expired or that name a + /// tombstone. + /// + /// + /// It replaces 0006's and 0007's trigger functions in place, so every + /// ENABLE ALWAYS binding survives and no table is unguarded for an instant. The guards keep + /// refusing UPDATE and TRUNCATE unconditionally and admit a DELETE only while + /// the purge function's transaction-scoped marker is set -- which is an auditability mechanism, not + /// a privilege boundary: a custom GUC is settable by any session, and the guards still do not bind a + /// role that can ALTER TABLE. + /// + /// It also creates the only two triggers it adds, experience_records_no_delete and + /// experience_records_no_truncate, which refuse removing a record row from every session with + /// no marker exception at all -- the erasure never deletes that row, and a freed + /// experience_id would let a recreated record inherit the old content's sharing grants. + /// + /// + /// The two purge functions are SECURITY DEFINER, so the script revokes EXECUTE on them + /// from PUBLIC -- PostgreSQL's default would otherwise make erasure reachable by every role + /// that can connect -- and grants it to the migrating role. An application role that is not the + /// migrating role needs an explicit grant. + /// + /// + /// Its three indexes are built with plain CREATE INDEX inside the migrator's per-script + /// transaction; the script's header carries the CONCURRENTLY runbook for building them out of + /// band first, the confirm-then-VALIDATE step, and the note that the erased text survives in + /// dead heap tuples until VACUUM. See the script's own header and the package README. + /// + /// + public const string DeleteAndExpireScriptName = "0010_delete_and_expire.sql"; + private const string ResourcePrefix = "AgentExperience.Storage.Postgres.Migrations."; /// @@ -118,6 +153,7 @@ public static class PostgresExperienceRecordSchema ConfidenceEvidenceScriptName, ReuseFeedbackScriptName, GrantAccessLogScriptName, + DeleteAndExpireScriptName, ]; /// Reads an embedded script's SQL text. diff --git a/src/AgentExperience.Storage.Postgres/PostgresExperienceRecordStore.cs b/src/AgentExperience.Storage.Postgres/PostgresExperienceRecordStore.cs index 26d50f8..8a71eed 100644 --- a/src/AgentExperience.Storage.Postgres/PostgresExperienceRecordStore.cs +++ b/src/AgentExperience.Storage.Postgres/PostgresExperienceRecordStore.cs @@ -23,11 +23,14 @@ namespace AgentExperience.Storage.Postgres; /// does a read the caller declared an -- it is about to refuse the /// record for being grant-readable, so nothing is handed over. The two search channels audit what they return /// through their own batched append. -/// is the only operation that changes a stored record: it appends +/// is the only operation that changes a live record: it appends /// the event and updates the record's projection in one transaction on one connection, keyed by /// for idempotency and by /// for concurrency. The store persists the transition Core /// decided and never derives a status, score, or counter of its own. +/// is the one destructive +/// operation: it erases a record's payload and every stored row that named it, in one transaction, and leaves a +/// payload-free tombstone behind that every other path then refuses. /// PostgreSQL timestamptz stores microseconds, so and /// are truncated to whole microseconds (in UTC) on write. /// Nested timestamps live in the JSONB payload at full precision and are also returned in UTC. @@ -43,6 +46,16 @@ public sealed class PostgresExperienceRecordStore : IExperienceRecordStore /// The canonical record table. Shared with , which reads from it. internal const string Table = "agent_experience.experience_records"; + /// The smallest batch accepts. There is no "sweep everything". + public const int MinSweepBatchSize = 1; + + /// + /// The largest batch accepts. Each record in a batch is erased in + /// its own transaction across seven tables, so the bound is what keeps one sweep call from becoming + /// an unbounded amount of destructive work the host cannot interrupt. + /// + public const int MaxSweepBatchSize = 500; + /// /// The record columns every read selects, in the order expects (ordinals 0-17). /// A reader that selects more must append its extra columns after these, never before. @@ -52,6 +65,59 @@ public sealed class PostgresExperienceRecordStore : IExperienceRecordStore "status, reuse_confidence, supporting_validations, contradictions, revision, created_at, updated_at, " + "payload_version, payload"; + /// + /// "this row is not a tombstone", unqualified, for a statement over the record table alone. + /// + /// A tombstone carries no payload at all (see 0010), so it is not a record a read can + /// return: would fail on it, and a list that included + /// one would be handing back a row with nothing in it. Every read filters it out, and the two + /// operations that name one record -- and + /// -- read deleted_at instead of filtering on it, so they can + /// answer inside the scope that owns the tombstone. + /// + /// + internal const string LivePredicate = "deleted_at IS NULL"; + + /// The same, qualified with the r alias, for a statement that joins the record table to another. + internal const string RecordLivePredicate = "r.deleted_at IS NULL"; + + /// + /// The row lock every write that gates on has to take on + /// the record row it gated against. + /// + /// Without it the predicate is evaluated against a READ COMMITTED snapshot taken before the erasure + /// committed, and the write lands on a record the caller has already been told is gone: a stored + /// vector derived from the erased summary and lesson, a live sharing grant over a spent ID, or a + /// reviewer identity and free-text rationale about an erased record in an append-only ledger. With + /// it, the writer is parked against the purge's own FOR UPDATE (step 1 of + /// purge_experience_record) and, when the purge commits, PostgreSQL re-checks the write's + /// predicate against the row version the purge left behind -- which is the tombstone, so the write + /// matches nothing and the caller is told it lost. The same lock taken first makes the purge wait + /// instead, and the erasure then sweeps the row the writer committed. + /// + /// + /// FOR KEY SHARE rather than FOR SHARE on purpose: it is the weakest mode that still + /// conflicts with the purge's FOR UPDATE, and it does not conflict with the + /// FOR NO KEY UPDATE an ordinary lifecycle commit's projection UPDATE takes, so + /// serializing against erasure costs nothing against the writes that happen all the time. + /// + /// + /// needs no locking clause of its own: its projection + /// UPDATE is itself the lock, and 0010's projection guard refuses any UPDATE of + /// a tombstone from the database's side as well. + /// + /// + internal const string RecordKeyShareLock = "FOR KEY SHARE OF r"; + + /// The alias a read selects deleted_at under, read back by name, never by ordinal. + internal const string DeletedAtAlias = "deleted_at"; + + /// + /// The tombstone marker, appended after the record columns so 's + /// ordinals 0-17 are untouched. + /// + internal const string DeletedAtColumn = "r." + DeletedAtAlias + " AS " + DeletedAtAlias; + /// The exact-scope predicate every statement applies, shared with . internal const string ScopePredicate = "tenant_id = @tenant_id AND application_id = @application_id AND project_id = @project_id " + @@ -76,7 +142,7 @@ public sealed class PostgresExperienceRecordStore : IExperienceRecordStore /// /// private const string GetSql = - $"SELECT {SelectColumns}, {SharedByGrantColumn}, {PermittingGrantColumn} FROM {Table} r " + + $"SELECT {SelectColumns}, {SharedByGrantColumn}, {PermittingGrantColumn}, {DeletedAtColumn} FROM {Table} r " + $"{PermittingGrantJoin} " + $"WHERE r.experience_id = @experience_id AND {ReadableWithNamedGrantPredicate}"; @@ -87,10 +153,11 @@ public sealed class PostgresExperienceRecordStore : IExperienceRecordStore /// requesting scope already owns. /// private const string GetExactSql = - $"SELECT {SelectColumns}, false AS {SharedByGrantAlias}, NULL::uuid AS {PermittingGrantAlias} FROM {Table} r " + + $"SELECT {SelectColumns}, false AS {SharedByGrantAlias}, NULL::uuid AS {PermittingGrantAlias}, {DeletedAtColumn} " + + $"FROM {Table} r " + $"WHERE r.experience_id = @experience_id AND {RecordScopePredicate}"; - private const string QuerySql = $"SELECT {SelectColumns} FROM {Table} WHERE {ScopePredicate}"; + private const string QuerySql = $"SELECT {SelectColumns} FROM {Table} WHERE {ScopePredicate} AND {LivePredicate}"; private const string QueryStatusPredicate = " AND status = ANY(@statuses)"; @@ -118,6 +185,9 @@ public sealed class PostgresExperienceRecordStore : IExperienceRecordStore /// The ordinal r.revision sits at in , straight after . private const int HistoryRevisionOrdinal = 32; + /// The ordinal r.deleted_at sits at in , straight after the revision. + private const int HistoryDeletedAtOrdinal = 33; + private const string InsertEventSql = $"INSERT INTO {EventsTable} ({EventColumns}) VALUES (@event_id, @experience_id, @tenant_id, @application_id, " + "@project_id, @team_id, @agent_id, @user_id, @prior_status, @current_status, @reason, @producer, " + @@ -188,7 +258,7 @@ public sealed class PostgresExperienceRecordStore : IExperienceRecordStore "ev.prior_reuse_confidence, ev.new_reuse_confidence, ev.prior_supporting_validations, " + "ev.new_supporting_validations, ev.prior_contradictions, ev.new_contradictions " + $"FROM {EvidenceTable} ev JOIN {Table} r ON r.experience_id = ev.experience_id " + - $"WHERE ev.evidence_id = @evidence_id AND {RecordScopePredicate}"; + $"WHERE ev.evidence_id = @evidence_id AND {RecordScopePredicate} AND {RecordLivePredicate}"; /// /// The record's revision and status, locked for the rest of the transaction. Used only on the @@ -222,17 +292,38 @@ public sealed class PostgresExperienceRecordStore : IExperienceRecordStore ", reuse_confidence = @new_reuse_confidence, supporting_validations = @new_supporting_validations, " + "contradictions = @new_contradictions"; + /// + /// The guards above plus "and this record has not been erased". A tombstone is terminal: a late + /// commit against one matches no row here and is reported as + /// after the same re-read that tells a stale revision + /// from a missing record. The database refuses it a second time from its own side -- 0010's + /// projection guard rejects every UPDATE of a tombstone -- so neither this adapter nor a writer + /// bypassing it can move one. + /// + /// The tombstone term here is redundant and kept on purpose: status = COALESCE(...) compares + /// against an member's name, and a tombstone's status is a literal no + /// member has, so this statement could never match one anyway. It is defence in depth against a + /// future status whose name collides, and it is named as redundant rather than counted as the thing + /// that makes late commits safe -- the re-read below, and 0010's projection guard, are. + /// + /// private const string UpdateProjectionWhereSql = " WHERE experience_id = @experience_id AND revision = @expected_revision " + - $"AND status = COALESCE(@prior_status, @current_status) AND {ScopePredicate}"; + $"AND status = COALESCE(@prior_status, @current_status) AND {ScopePredicate} AND {LivePredicate}"; private const string UpdateProjectionSql = UpdateProjectionSetSql + UpdateProjectionWhereSql; private const string UpdateProjectionWithConfidenceSql = UpdateProjectionSetSql + UpdateProjectionConfidenceSetSql + UpdateProjectionWhereSql; + /// + /// The record's revision, status, and tombstone marker within exactly this scope. deleted_at + /// is selected rather than filtered on, because this read is what turns "the guarded UPDATE matched + /// no row" into a reason, and "erased" is one of the reasons. The status of a tombstone is a literal + /// this library's enum has no member for, so it is only ever decoded when deleted_at is null. + /// private const string SelectRevisionAndStatusSql = - $"SELECT revision, status FROM {Table} WHERE experience_id = @experience_id AND {ScopePredicate}"; + $"SELECT revision, status, {DeletedAtAlias} FROM {Table} WHERE experience_id = @experience_id AND {ScopePredicate}"; private const string SelectEventSql = $"SELECT {EventColumns} FROM {EventsTable} WHERE event_id = @event_id"; @@ -397,7 +488,7 @@ public sealed class PostgresExperienceRecordStore : IExperienceRecordStore /// /// private const string HistorySql = - $"SELECT {JoinedEventColumns}, r.revision FROM {Table} r " + + $"SELECT {JoinedEventColumns}, r.revision, r.{DeletedAtAlias} FROM {Table} r " + $"LEFT JOIN {EventsTable} e ON e.experience_id = r.experience_id " + "AND (@start_after_revision IS NULL OR e.applied_revision > @start_after_revision) " + $"WHERE r.experience_id = @experience_id AND {RecordScopePredicate} " + @@ -448,11 +539,48 @@ SELECT e.replacement_experience_id WHERE e.replacement_experience_id IS NOT NULL AND {EventScopePredicate} ) SELECT - (SELECT r.status FROM {Table} r WHERE r.experience_id = @experience_id AND {RecordScopePredicate}), - (SELECT r.status FROM {Table} r WHERE r.experience_id = @replacement_id AND {RecordScopePredicate}), + (SELECT r.status FROM {Table} r + WHERE r.experience_id = @experience_id AND {RecordScopePredicate} AND {RecordLivePredicate}), + (SELECT r.status FROM {Table} r + WHERE r.experience_id = @replacement_id AND {RecordScopePredicate} AND {RecordLivePredicate}), EXISTS (SELECT 1 FROM replaced_by WHERE experience_id = @experience_id) """; + /// + /// The one erasure path, created by 0010. Every step of it -- the scope and revision guards, + /// the seven tables it sweeps, and the tombstone it leaves -- runs inside this one function, in one + /// transaction, under a marker the append-only guards recognise and no other session can see. This + /// adapter composes no DELETE of its own: there is nothing here to get out of step with the order the + /// script pins. + /// + private const string PurgeSql = + "SELECT purge_outcome, purge_revision FROM agent_experience.purge_experience_record(" + + "@experience_id, @tenant_id, @application_id, @project_id, @team_id, @agent_id, @user_id, " + + "@expected_revision, @deleted_at)"; + + /// + /// One bounded page of a scope's records that are older than the retention cutoff, oldest first. + /// Deliberately only the IDs: the sweep erases what it finds and never reads a payload it is about + /// to destroy. One row beyond the batch is selected so the result can say whether more remain + /// without a second count. + /// + private const string SweepCandidatesSql = + $"SELECT experience_id FROM {Table} " + + $"WHERE {ScopePredicate} AND {LivePredicate} AND created_at < @cutoff " + + "ORDER BY created_at, experience_id LIMIT @limit"; + + /// The purge function's outcome for a record it erased. + private const string PurgedOutcome = "Deleted"; + + /// The purge function's outcome for a record that was already a tombstone. + private const string AlreadyPurgedOutcome = "AlreadyDeleted"; + + /// The purge function's outcome for a record that is not in the requesting scope, erased or not. + private const string PurgeNotFoundOutcome = "NotFound"; + + /// The purge function's outcome for a record whose revision has moved past the expected one. + private const string PurgeStaleOutcome = "StaleRevision"; + private static readonly IReadOnlyList NoErrors = []; /// What the four-argument means: a caller that keeps what it reads. @@ -464,6 +592,8 @@ WHERE e.replacement_experience_id IS NOT NULL AND {EventScopePredicate} private readonly ExperienceGrantAuditing? _auditing; + private readonly TimeProvider _timeProvider; + /// Creates a store over a host-owned data source. The store never disposes it. /// The Npgsql data source to open connections from. /// @@ -476,16 +606,25 @@ WHERE e.replacement_experience_id IS NOT NULL AND {EventScopePredicate} /// -- the default -- switches auditing off entirely: no extra write, no /// extra failure mode, and a deployment behaves exactly as it did before this was added. /// + /// + /// The clock this store stamps its own readings from: a lifecycle event's recorded_at, a + /// tombstone's deleted_at, and the cutoff a retention sweep measures against + /// . Defaults to . It is + /// this store's own clock and never a caller's: whether a sharing grant is still live is always the + /// database's clock_timestamp(), which no host can wind. + /// /// is . public PostgresExperienceRecordStore( NpgsqlDataSource dataSource, Action? onGrantsUnavailable = null, - ExperienceGrantAuditing? auditing = null) + ExperienceGrantAuditing? auditing = null, + TimeProvider? timeProvider = null) { ArgumentNullException.ThrowIfNull(dataSource); _dataSource = dataSource; _grants = new PostgresGrantSupport(onGrantsUnavailable); _auditing = auditing; + _timeProvider = timeProvider ?? TimeProvider.System; } /// @@ -639,11 +778,26 @@ private async Task ReadOneAsync( return new(ExperienceStoreOutcome.NotFound, null, NoErrors); } + var sharedByGrant = ReadSharedByGrant(reader); + + if (ReadDeleted(reader)) + { + // An erased record. The owner is told so -- the ID is spent and no retry will make it + // resolve -- but a reader that only reached the row through a grant is told nothing it did + // not already have: the erasure purges every grant over the record, so a grant that still + // names a tombstone was written outside this library, and answering it with anything but + // NotFound would leak the tombstone's existence across a scope boundary. + return new( + sharedByGrant ? ExperienceStoreOutcome.NotFound : ExperienceStoreOutcome.Deleted, + null, + NoErrors); + } + return new( ExperienceStoreOutcome.Found, ReadRecord(reader), NoErrors, - ReadSharedByGrant(reader), + sharedByGrant, ReadPermittingGrant(reader)); } @@ -774,7 +928,7 @@ public async Task CommitLifecycleEventAsync( // Both timestamps are truncated the same way the record's columns are, so a replay's stored // OccurredAt compares equal to the value the caller resubmits. var occurredAt = ToStoredTimestamp(lifecycleEvent.OccurredAt); - var recordedAt = ToStoredTimestamp(DateTimeOffset.UtcNow); + var recordedAt = ToStoredTimestamp(_timeProvider.GetUtcNow()); var appliedRevision = lifecycleEvent.ExpectedRevision + 1; try @@ -908,6 +1062,13 @@ public async Task CommitLifecycleEventAsync( return new(ExperienceStoreOutcome.NotFound, 0, null, NoErrors); } + if (record.Deleted) + { + // The record was erased. A tombstone is terminal, so this is not a race to retry: + // the event this call appended is rolled back with everything else. + return new(ExperienceStoreOutcome.Deleted, record.Revision, null, NoErrors); + } + return record.Revision != lifecycleEvent.ExpectedRevision ? new(ExperienceStoreOutcome.StaleRevision, record.Revision, null, NoErrors) // Scope and revision both matched, so the prior-status guard is what rejected it. @@ -967,6 +1128,14 @@ public async Task GetHistoryAsync( return new(ExperienceStoreOutcome.NotFound, 0, [], NoErrors); } + if (!reader.IsDBNull(HistoryDeletedAtOrdinal)) + { + // An erased record has no history left to page: its events were removed with its + // payload. Reported as Deleted rather than as an empty page, which would say the record + // is alive and has nothing to show. + return new(ExperienceStoreOutcome.Deleted, 0, [], NoErrors); + } + var revision = ReadRevision(reader, HistoryRevisionOrdinal); var events = new List(); @@ -1031,6 +1200,307 @@ public async Task CheckSupersessionAsync( } } + /// + /// Erases one record: its payload and every stored row that named it, leaving a payload-free + /// tombstone under the same ID. This is the only destructive operation this library has. + /// + /// + /// + /// What is erased, and what is left. The evidence ledger, the exposure rows, the grants and + /// their audit events, the lifecycle history, and the embedding are removed. The record row survives + /// carrying only , the six scope columns, + /// , the deletion timestamp, a tombstone status, and a fixed + /// task_id placeholder. experience_grant_access rows are deliberately kept: they name + /// a grant and a principal, carry no payload, and are the answer to "who read this before it was + /// deleted". See the package README for the retained list, stated exhaustively. + /// + /// + /// One transaction, one code path. Every step runs inside 0010's + /// purge_experience_record function, in the order that script pins, under a + /// transaction-scoped marker the append-only guards recognise. The guards are never disabled and + /// never widened for another session. That buys atomicity and a single path -- not a privilege + /// boundary; the README says exactly what it does not bind. + /// + /// + /// Foreign scope is indistinguishable from absent, exactly as it is everywhere else: both are + /// , decided by one statement's predicate rather than + /// by a branch here. Deleting twice is again, + /// with nothing written. + /// + /// + /// This is not on . Erasure is a capability of this + /// adapter, not of the port: Core never deletes, and a port method would oblige every + /// implementation -- including the in-memory doubles hosts write for tests -- to promise an erasure + /// it cannot actually perform. + /// + /// + /// What the host has established the caller may do. + /// The exact scope the record must lie in. Never treated as authority. + /// The record to erase. Must not be . + /// Cancels the operation. + /// , , , or . + /// or is . + public Task DeleteAsync( + AuthorizationContext authorization, + Scope scope, + Guid experienceId, + CancellationToken cancellationToken) => + DeleteAsync(authorization, scope, experienceId, expectedRevision: null, cancellationToken); + + /// + /// The same erasure, refused unless the record is still at . + /// + /// + /// The revision guard, the scope predicate, and the existence check are one statement inside the + /// purge function, so a stale revision, a foreign scope, and a missing record are all "no row" -- + /// and only the scope that owns the record is told which. Pass to erase + /// whatever revision the record is at, which is what the retention sweep does: an age-based deletion + /// is not racing a writer for a particular version. + /// + /// What the host has established the caller may do. + /// The exact scope the record must lie in. Never treated as authority. + /// The record to erase. Must not be . + /// The revision the record must still be at, or for none. Must not be negative. + /// Cancels the operation. + /// + /// , + /// (carrying the record's current revision), , + /// , or . + /// + /// or is . + public async Task DeleteAsync( + AuthorizationContext authorization, + Scope scope, + Guid experienceId, + long? expectedRevision, + CancellationToken cancellationToken) + { + ArgumentNullException.ThrowIfNull(authorization); + ArgumentNullException.ThrowIfNull(scope); + + var errors = ExperienceRecordValidator.ValidateDelete(scope, experienceId, expectedRevision); + if (errors.Count > 0) + { + return new(ExperienceStoreOutcome.Invalid, 0, errors); + } + + if (!authorization.Permits(scope)) + { + // Fail-closed, and before any connection opens: nothing is erased and nothing is read. + return new(ExperienceStoreOutcome.Denied, 0, NoErrors); + } + + cancellationToken.ThrowIfCancellationRequested(); + + try + { + await using var connection = await _dataSource.OpenConnectionAsync(cancellationToken).ConfigureAwait(false); + return await PurgeAsync(connection, scope, experienceId, expectedRevision, cancellationToken).ConfigureAwait(false); + } + catch (Exception ex) when (IsInfrastructureFailure(ex, cancellationToken)) + { + throw Translate(ex, "delete", cancellationToken); + } + } + + /// + /// Erases the records in one scope that are older than , in one + /// bounded batch, through exactly the same erasure as . + /// + /// + /// + /// There is no default retention and no timer. Nothing expires unless a host calls this with + /// a positive age, and this library ships no scheduler, no background service, and no hosted + /// service: when a sweep runs is the host's decision, made with the host's own scheduling, because + /// only the host knows what its data-retention obligations are. + /// + /// + /// Bounded, and resumable. At most records are erased per call, + /// oldest first, and + /// says whether another call would find + /// more. Each record is erased in its own transaction, so an interrupted sweep leaves every record + /// it reached wholly erased and every record it did not reach wholly untouched. + /// + /// + /// The cutoff is measured on this store's against the record's stored + /// , never against : + /// age is how long the library has held the data, and a record that is read, ranked, or re-scored + /// does not thereby become younger. + /// + /// + /// It sweeps the EXACT scope and no scope under it, and that is the one failure mode here that + /// looks like success. is matched field for field, exactly as every + /// other operation in this library matches it, so a sweep of + /// (tenant, app, project) with no team, agent or user reaches only the records stored with + /// those three fields and all three of the others null. Records the same tenant holds under a team, + /// an agent or a user are a different scope: they are not swept, they are not counted, and + /// comes back -- + /// a retention obligation quietly unmet, reported as a clean sweep. A host whose policy is + /// "delete everything in this tenant older than N days" must enumerate every leaf scope it has + /// written under and call this once per scope; the library cannot enumerate them for it, because a + /// scope is the host's own partitioning and nothing here knows which of them exist. + /// + /// + /// Stopping early. Cancelling between records returns what the call had already erased, with + /// set, rather than throwing away the + /// count. A storage failure part-way through throws + /// , which carries the same partial + /// result and is an like any other storage failure here. + /// + /// + /// What the host has established the caller may do. + /// The exact scope to sweep, matched field for field. Never treated as authority, and never widened to the scopes beneath it. + /// How long a record may be kept, measured from . Must be strictly positive. + /// The most records this call may erase, from to . + /// Cancels the operation between records; records already erased stay erased, and the count comes back on the result rather than being lost. + /// + /// when the sweep ran (possibly erasing nothing, and + /// possibly stopping early -- see ), + /// , or . + /// + /// or is . + /// A storage failure stopped the batch part-way; the count of what was erased is on the exception. + public async Task SweepExpiredAsync( + AuthorizationContext authorization, + Scope scope, + TimeSpan retentionAge, + int batchSize, + CancellationToken cancellationToken) + { + ArgumentNullException.ThrowIfNull(authorization); + ArgumentNullException.ThrowIfNull(scope); + + var errors = ExperienceRecordValidator.ValidateRetentionSweep(scope, retentionAge, batchSize); + if (errors.Count > 0) + { + return new(ExperienceStoreOutcome.Invalid, 0, false, errors); + } + + if (!authorization.Permits(scope)) + { + return new(ExperienceStoreOutcome.Denied, 0, false, NoErrors); + } + + cancellationToken.ThrowIfCancellationRequested(); + + var cutoff = ToStoredTimestamp(_timeProvider.GetUtcNow() - retentionAge); + + try + { + await using var connection = await _dataSource.OpenConnectionAsync(cancellationToken).ConfigureAwait(false); + + var candidates = new List(batchSize + 1); + await using (var command = new NpgsqlCommand(SweepCandidatesSql, connection)) + { + AddScopeParameters(command.Parameters, scope); + command.Parameters.Add(new NpgsqlParameter("cutoff", cutoff)); + + // One row beyond the batch, so "more remain" is read off the same statement rather than + // from a second count that could disagree with it. + command.Parameters.Add(new NpgsqlParameter("limit", batchSize + 1)); + + await using var reader = await command.ExecuteReaderAsync(cancellationToken).ConfigureAwait(false); + while (await reader.ReadAsync(cancellationToken).ConfigureAwait(false)) + { + candidates.Add(reader.GetGuid(0)); + } + } + + var moreRemain = candidates.Count > batchSize; + + // Declared outside the loop, and read again by both handlers below, because the number of + // records this call irreversibly erased is the one fact a compliance log needs and it must + // not be lost just because the batch stopped early. + var deleted = 0; + try + { + foreach (var experienceId in candidates.Take(batchSize)) + { + // No expected revision: a sweep deletes a record for its age, not for the version it + // happened to be at when the page was read. + var result = await PurgeAsync(connection, scope, experienceId, expectedRevision: null, cancellationToken) + .ConfigureAwait(false); + + if (result.Outcome == ExperienceStoreOutcome.Deleted) + { + deleted++; + } + } + } + catch (Exception ex) when (ex is not ExperienceStoreException && cancellationToken.IsCancellationRequested) + { + // Caller cancellation, however the driver reported it -- an OperationCanceledException, + // or the server's own query_canceled for a statement that was already running. Decided + // by the token exactly as Translate decides it, so the two never disagree. + // + // The host asked the sweep to stop, which is a normal way to run one: each record was + // erased in its own transaction, so what is erased is erased and what is left is whole. + // Returned rather than thrown, because a cancelled sweep that threw away its count would + // leave a host unable to say how much of its data it had just destroyed. MoreRemain is + // true regardless of what the page said: at least the record it stopped on is still there. + return new(ExperienceStoreOutcome.Deleted, deleted, true, NoErrors, Interrupted: true); + } + catch (Exception ex) when (IsInfrastructureFailure(ex, cancellationToken)) + { + // A failure, not a request. Thrown -- a host must not read this as a sweep that ran -- + // but thrown carrying the count, as an ExperienceStoreException like every other storage + // failure here, so nothing that already catches those has to change. + throw new ExperienceRetentionSweepInterruptedException( + new(ExperienceStoreOutcome.Deleted, deleted, true, NoErrors, Interrupted: true), + Translate(ex, "retention sweep", cancellationToken)); + } + + return new(ExperienceStoreOutcome.Deleted, deleted, moreRemain, NoErrors); + } + catch (Exception ex) when (IsInfrastructureFailure(ex, cancellationToken)) + { + // Only the candidate read can reach this now, and it erases nothing. + throw Translate(ex, "retention sweep", cancellationToken); + } + } + + /// + /// Runs the purge function and maps its outcome. One statement, so the whole erasure is one + /// transaction whether or not the caller opened one. + /// + private async Task PurgeAsync( + NpgsqlConnection connection, + Scope scope, + Guid experienceId, + long? expectedRevision, + CancellationToken cancellationToken) + { + await using var command = new NpgsqlCommand(PurgeSql, connection); + var parameters = command.Parameters; + parameters.Add(new NpgsqlParameter("experience_id", experienceId)); + AddScopeParameters(parameters, scope); + parameters.Add(new NpgsqlParameter("expected_revision", NpgsqlDbType.Bigint) + { + Value = expectedRevision is { } revision ? revision : DBNull.Value, + }); + parameters.Add(new NpgsqlParameter("deleted_at", ToStoredTimestamp(_timeProvider.GetUtcNow()))); + + await using var reader = await command.ExecuteReaderAsync(cancellationToken).ConfigureAwait(false); + if (!await reader.ReadAsync(cancellationToken).ConfigureAwait(false)) + { + // The function always returns exactly one row; treat the impossible case the way a missing + // record is treated, which writes nothing and claims nothing. + return new(ExperienceStoreOutcome.NotFound, 0, NoErrors); + } + + var outcome = reader.GetString(0); + var currentRevision = reader.GetInt64(1); + + return outcome switch + { + // Erased now, or erased earlier: deleting twice is a success that touches nothing. + PurgedOutcome or AlreadyPurgedOutcome => new(ExperienceStoreOutcome.Deleted, currentRevision, NoErrors), + PurgeStaleOutcome => new(ExperienceStoreOutcome.StaleRevision, currentRevision, NoErrors), + PurgeNotFoundOutcome => new(ExperienceStoreOutcome.NotFound, 0, NoErrors), + _ => throw new ExperienceStoreException("The erasure function reported an unrecognized outcome."), + }; + } + /// /// Re-decides the replacement rules inside the commit transaction, with both record rows locked, and /// returns the refusal when they no longer hold. means the supersession may @@ -1284,6 +1754,13 @@ async Task InsertOneAsync(ConfidenceUpdate update, Guid? eventId, long revision, return (new(ExperienceStoreOutcome.NotFound, 0, null, NoErrors), Commit: false); } + if (record.Deleted) + { + // Nothing is recorded against a tombstone -- not even a duplicate submission's ledger + // row, which would put the erased record's ID back into a table the erasure emptied. + return (new(ExperienceStoreOutcome.Deleted, record.Revision, null, NoErrors), Commit: false); + } + if (record.Revision != lifecycleEvent.ExpectedRevision) { return (new(ExperienceStoreOutcome.StaleRevision, record.Revision, null, NoErrors), Commit: false); @@ -1297,7 +1774,9 @@ async Task InsertOneAsync(ConfidenceUpdate update, Guid? eventId, long revision, var recordedOnly = submitted.AsRecordedOnly(); try { - await InsertOneAsync(recordedOnly, eventId: null, record.Revision, record.Status).ConfigureAwait(false); + // Not a tombstone, so the status decoded: the branch above returned for the one case + // where it could not. + await InsertOneAsync(recordedOnly, eventId: null, record.Revision, record.Status!.Value).ConfigureAwait(false); } catch (PostgresException pk) when (IsViolationOf(pk, EvidencePrimaryKey, cancellationToken)) { @@ -1426,9 +1905,14 @@ private static async Task StaleOrMissingAsync( CancellationToken cancellationToken) { var current = await ReadRevisionAndStatusAsync(connection, transaction, scope, experienceId, cancellationToken).ConfigureAwait(false); - return current is { } record - ? new(ExperienceStoreOutcome.StaleRevision, record.Revision, null, NoErrors) - : new(ExperienceStoreOutcome.NotFound, 0, null, NoErrors); + if (current is not { } record) + { + return new(ExperienceStoreOutcome.NotFound, 0, null, NoErrors); + } + + return record.Deleted + ? new(ExperienceStoreOutcome.Deleted, record.Revision, null, NoErrors) + : new(ExperienceStoreOutcome.StaleRevision, record.Revision, null, NoErrors); } /// @@ -1436,7 +1920,7 @@ private static async Task StaleOrMissingAsync( /// the row for the rest of the transaction. Only the duplicate path needs the lock: every other caller /// either holds the row through its own revision-guarded UPDATE or is reporting a race it already lost. /// - private static async Task<(long Revision, ExperienceStatus Status)?> ReadRevisionAndStatusAsync( + private static async Task ReadRevisionAndStatusAsync( NpgsqlConnection connection, NpgsqlTransaction? transaction, Scope scope, @@ -1455,9 +1939,19 @@ private static async Task StaleOrMissingAsync( return null; } - return (ReadRevision(reader, 0), ReadStoredStatus(reader, 1)); + // A tombstone's status is a literal no ExperienceStatus member names, so it is never decoded: + // the marker is read first and the status left alone. + return reader.IsDBNull(2) + ? new StoredRecordState(ReadRevision(reader, 0), ReadStoredStatus(reader, 1), Deleted: false) + : new StoredRecordState(ReadRevision(reader, 0), null, Deleted: true); } + /// + /// What a scoped read of one record row found: its revision, its status when it has one this + /// library's enum names, and whether it is a tombstone. + /// + private readonly record struct StoredRecordState(long Revision, ExperienceStatus? Status, bool Deleted); + /// /// Matches a unique violation of one named constraint. Naming it keeps the event primary key (a /// resubmitted event ID) apart from the record-revision index (a lost race), so neither is ever @@ -1616,6 +2110,23 @@ internal static bool ReadSharedByGrant(DbDataReader reader) } } + /// + /// Reads the tombstone marker by name. A reader that did not select it is treated as "not erased", + /// which is the safe direction for a caller that never asked: every statement that could meet a + /// tombstone either selects this column or filters tombstones out in SQL. + /// + internal static bool ReadDeleted(DbDataReader reader) + { + try + { + return !reader.IsDBNull(reader.GetOrdinal(DeletedAtAlias)); + } + catch (IndexOutOfRangeException) + { + return false; + } + } + internal static ExperienceRecord ReadRecord(DbDataReader reader) { try diff --git a/src/AgentExperience.Storage.Postgres/PostgresExperienceReuseFeedbackStore.cs b/src/AgentExperience.Storage.Postgres/PostgresExperienceReuseFeedbackStore.cs index d67a534..d2a6c11 100644 --- a/src/AgentExperience.Storage.Postgres/PostgresExperienceReuseFeedbackStore.cs +++ b/src/AgentExperience.Storage.Postgres/PostgresExperienceReuseFeedbackStore.cs @@ -37,10 +37,16 @@ namespace AgentExperience.Storage.Postgres; /// row in another scope and handing it back would be a cross-scope read. /// /// -/// This store never reads an Experience Record. There is no foreign key to -/// experience_records and no join to it: an exposed ID that resolves to nothing in the scope is -/// recorded exactly like one that resolves, and whether it resolves is decided later, by the confidence -/// path, against the record itself. +/// This store reads an Experience Record for exactly one reason. There is still no foreign key +/// to experience_records and no join in the write: an exposed ID that resolves to nothing in the +/// scope is recorded exactly like one that resolves, and whether it resolves is decided later, by the +/// confidence path, against the record itself. The one exception is an erased record. Recording +/// an exposure to a tombstone would write the ID back into a ledger the erasure emptied, so a submission +/// naming one is , naming the exposure by +/// position, with nothing written. Only tombstones in the submission's own scope are visible to that +/// check, so a refusal can never reveal another scope's. That read takes FOR KEY SHARE on the +/// records it names, so a record erased while the submission is being written is refused rather than +/// recorded against. /// /// /// The database's own CHECKs are defence in depth, not a second validation path. Every rule @@ -102,17 +108,54 @@ public sealed class PostgresExperienceReuseFeedbackStore : IExperienceReuseFeedb $"SELECT experience_id, attributed, evidence_id FROM {ExposuresTable} " + "WHERE feedback_id = @feedback_id ORDER BY ordinal"; + /// + /// Every exposed record this scope actually holds, with its tombstone marker, locked for the rest of + /// the transaction. + /// + /// A run's exposure to a record that never existed here, or that was revoked, is still recordable -- + /// 0008 has no foreign key precisely so that "the run saw an ID that resolves to nothing" stays + /// a fact worth keeping. An erased record is the one exception: writing its ID into this + /// ledger would put back an association the erasure just removed, and a human assessment carries a + /// reviewer identity and a free-text rationale about the record into an append-only table. A + /// tombstone in another scope is invisible to this statement, so a refusal can never reveal one. + /// + /// + /// It selects live rows too, and locks them, on purpose. Asking only for tombstones would + /// lock nothing when every named record is still live -- which is the case a concurrent erasure + /// turns into a lie between this read and the exposure inserts below it. Taking + /// FOR KEY SHARE over every named record in scope, live or not, is what makes this a check + /// that holds until the transaction commits rather than a check-then-write; see + /// . The rows are locked in + /// experience_id order so two submissions naming overlapping records cannot deadlock. + /// + /// + private static readonly string SelectExposedRecordStateSql = + $"SELECT r.experience_id, r.{PostgresExperienceRecordStore.DeletedAtAlias} " + + $"FROM {PostgresExperienceRecordStore.Table} r " + + "WHERE r.experience_id = ANY(@experience_ids) " + + $"AND {PostgresExperienceRecordStore.RecordScopePredicate} " + + "ORDER BY r.experience_id " + + PostgresExperienceRecordStore.RecordKeyShareLock; + private static readonly IReadOnlyList NoErrors = []; private readonly NpgsqlDataSource _dataSource; + private readonly TimeProvider _timeProvider; + /// Creates a feedback store over a host-owned data source. The store never disposes it. /// The Npgsql data source to open connections from. + /// + /// The clock this store stamps recorded_at from -- its own reading of when the row landed, + /// deliberately separate from the caller's . + /// Defaults to . + /// /// is . - public PostgresExperienceReuseFeedbackStore(NpgsqlDataSource dataSource) + public PostgresExperienceReuseFeedbackStore(NpgsqlDataSource dataSource, TimeProvider? timeProvider = null) { ArgumentNullException.ThrowIfNull(dataSource); _dataSource = dataSource; + _timeProvider = timeProvider ?? TimeProvider.System; } /// @@ -171,6 +214,16 @@ public async Task RecordAsync( NoErrors); } + var erased = await ReadErasedExposuresAsync(connection, transaction, feedback, cancellationToken).ConfigureAwait(false); + if (erased.Count > 0) + { + // Read and refused inside the transaction that is about to be rolled back, so a + // submission naming an erased record writes nothing at all -- not the submission row the + // insert above provisionally took, and not one exposure. + await transaction.RollbackAsync(CancellationToken.None).ConfigureAwait(false); + return new(ExperienceReuseFeedbackStoreOutcome.Invalid, null, erased); + } + for (var ordinal = 0; ordinal < feedback.Exposures.Count; ordinal++) { await InsertExposureAsync(connection, transaction, feedback.FeedbackId, feedback.Exposures[ordinal], ordinal, cancellationToken) @@ -186,7 +239,7 @@ await InsertExposureAsync(connection, transaction, feedback.FeedbackId, feedback } } - private static async Task InsertSubmissionAsync( + private async Task InsertSubmissionAsync( NpgsqlConnection connection, NpgsqlTransaction transaction, RecordedExperienceReuseFeedback feedback, @@ -227,7 +280,7 @@ private static async Task InsertSubmissionAsync( PostgresExperienceRecordStore.ToStoredTimestamp(feedback.ObservedAt))); parameters.Add(new NpgsqlParameter( "recorded_at", - PostgresExperienceRecordStore.ToStoredTimestamp(DateTimeOffset.UtcNow))); + PostgresExperienceRecordStore.ToStoredTimestamp(_timeProvider.GetUtcNow()))); return await command.ExecuteScalarAsync(cancellationToken).ConfigureAwait(false) is not null; } @@ -252,6 +305,63 @@ private static async Task InsertExposureAsync( await command.ExecuteNonQueryAsync(cancellationToken).ConfigureAwait(false); } + /// + /// Names every exposure whose record has been erased, as a validation error per exposure. The + /// message is content-free and the path is an index, so a refusal says which position of the + /// caller's own submission is unrecordable without echoing anything back. + /// + private static async Task> ReadErasedExposuresAsync( + NpgsqlConnection connection, + NpgsqlTransaction transaction, + RecordedExperienceReuseFeedback feedback, + CancellationToken cancellationToken) + { + var exposedIds = feedback.Exposures.Select(exposure => exposure.ExperienceId).Distinct().ToArray(); + if (exposedIds.Length == 0) + { + return NoErrors; + } + + var erased = new HashSet(); + await using (var command = new NpgsqlCommand(SelectExposedRecordStateSql, connection, transaction)) + { + command.Parameters.Add(new NpgsqlParameter("experience_ids", NpgsqlDbType.Array | NpgsqlDbType.Uuid) + { + TypedValue = exposedIds, + }); + PostgresExperienceRecordStore.AddScopeParameters(command.Parameters, feedback.Scope); + + await using var reader = await command.ExecuteReaderAsync(cancellationToken).ConfigureAwait(false); + while (await reader.ReadAsync(cancellationToken).ConfigureAwait(false)) + { + // Every named record in scope comes back and is now locked; only the tombstones among + // them are refusals. + if (!reader.IsDBNull(1)) + { + erased.Add(reader.GetGuid(0)); + } + } + } + + if (erased.Count == 0) + { + return NoErrors; + } + + var errors = new List(); + for (var ordinal = 0; ordinal < feedback.Exposures.Count; ordinal++) + { + if (erased.Contains(feedback.Exposures[ordinal].ExperienceId)) + { + errors.Add(new( + $"Exposures[{ordinal}].ExperienceId", + "names an Experience Record that has been erased; nothing may be recorded against it again.")); + } + } + + return errors; + } + /// /// Reads the submission stored under this feedback ID, with its exposures in stored order. Read /// inside the caller's transaction, which is then rolled back, so a comparison can never be the diff --git a/src/AgentExperience.Storage.Postgres/README.md b/src/AgentExperience.Storage.Postgres/README.md index 28553f2..07150e2 100644 --- a/src/AgentExperience.Storage.Postgres/README.md +++ b/src/AgentExperience.Storage.Postgres/README.md @@ -238,11 +238,13 @@ the transaction opens, exactly as for the store's other operations, and the scop access. Treat this as a guard against a bug, a careless script, a compromised application path, or a replication apply — not as tamper-proofing against an administrator. A deployment that needs more should ship the log off-box, or own these tables with a role the application does not have. -- **Purging, until story 4.5.** Nothing can delete an event row now, and the logs carry free-text `reason` and - `producer` a host may have filled with personal data. The owner purges explicitly — `DISABLE TRIGGER`, a narrow - `DELETE`, `ENABLE ALWAYS TRIGGER`, all in one transaction so the guard is never off across a failure — and - reconciles `experience_records` afterwards, because deleting an event does not move the projection. `0006`'s - header carries the exact statements. +- **Purging is the one exception, and it never disables anything.** The logs carry free-text `reason` and + `producer` a host may have filled with personal data, so `0010` replaced `0006`'s manual + `DISABLE TRIGGER` runbook with a single purge function whose transaction-scoped marker the guards themselves + recognise: `UPDATE` and `TRUNCATE` stay refused in every session, `DELETE` is admitted only inside that one + function, and no other connection's window is widened for an instant. What that does and does not buy is stated + in full under [Deleting and expiring data](#deleting-and-expiring-data) — it is a single code path, not a + privilege boundary. ## Confidence evidence @@ -608,6 +610,271 @@ registers its own `IExperienceRecordStore`, `IExperienceCandidateSource`, or `IE calling these extensions keeps its own — and takes on the obligation to honour a configured `ExperienceGrantAuditing` itself. Registering the access log does not make somebody else's store audit. +## Deleting and expiring data + +Revocation stops reads. **Deletion removes payload.** `DeleteAsync` is the only destructive operation this library +has, and every choice in it is resolved towards "a wrong delete is refused" rather than "a right delete is +convenient". + +```csharp +var store = new PostgresExperienceRecordStore(dataSource); + +// Erase one record, in exactly this scope. +var deleted = await store.DeleteAsync(hostAuthorization, scope, experienceId, cancellationToken); +// deleted.Outcome is Deleted, NotFound, Invalid, or Denied. Deleting again is Deleted, and writes nothing. + +// Or erase it only while it is still at the revision you read. +var guarded = await store.DeleteAsync(hostAuthorization, scope, experienceId, expectedRevision: 4, cancellationToken); +// StaleRevision, carrying the record's current revision, when it has moved on. +``` + +**Deletion is payload erasure plus a tombstone, never a row vanishing.** The `experience_records` row survives with +its payload emptied; everything else that named the record is removed. That is what makes the ID unusable +afterwards rather than free to be written again. + +### What is retained after a delete, exhaustively + +| Column | Why it stays | +| --- | --- | +| `experience_id` | The tombstone itself: an opaque ID nothing can re-create a record under | +| `tenant_id`, `application_id`, `project_id`, `team_id`, `agent_id`, `user_id` | The scope, so the tombstone stays answerable to — and only to — the scope that owned it | +| `revision` | Erasure advances it once, like any other change, so a stale write still loses | +| `deleted_at` | When it was erased | +| `status` | The fixed literal `'Deleted'`. No `ExperienceStatus` member names it, and no read decodes it | +| `task_id` | The fixed literal `'(deleted)'`. The column is `NOT NULL` with a non-blank `CHECK`, so it cannot be emptied | +| `payload_version` | Unchanged. It describes the (now empty) payload envelope's shape and says nothing about the record — but it *does* survive, so it belongs on a list that calls itself exhaustive | + +**Nothing else.** `payload` becomes `'{}'`, `source_run_id` becomes the empty UUID, `reuse_confidence`, +`supporting_validations` and `contradictions` become `0`, and `created_at` and `updated_at` are set to `deleted_at` +— a tombstone's only timestamp is the moment it was erased, so it cannot say when the work happened. +`search_vector` is `GENERATED ALWAYS` from `task_id` and two payload fields, so it regenerates from the +placeholder alone and the record's searchable text is gone without any separate index maintenance. + +That last sentence is a property of `purge_experience_record` and of every tombstone this library made — not +something the schema can prove about a row somebody else inserted. The tombstone-shape `CHECK` constrains an +existing row's *shape*; a row INSERTed directly as a tombstone can carry any `created_at` it likes, because there +is no `UPDATE` for the projection guard to refuse. The same goes for its revision. + +### What one delete removes + +One transaction, this order, all inside `0010`'s `agent_experience.purge_experience_record`: + +| # | Table | Why here | +| --- | --- | --- | +| 1 | `experience_records` | `SELECT … FOR UPDATE` with the scope and revision guards. Authorization, and the row pinned for the rest | +| 2 | `confidence_evidence` | **Before the tombstone.** It has no scope columns and no foreign key, so the record row's scope is the only thing that makes it reachable by scope at all | +| 3 | `reuse_feedback_exposures` | Children before parents: the foreign key to `reuse_feedback` is `NO ACTION` | +| 4 | `reuse_feedback` | Only the submissions step 3 emptied. One that also named other records keeps its row and loses only this exposure | +| 5 | `experience_grant_events` | Before the grants: the audited-delete guard refuses a grant that still has events | +| 6 | `experience_grants` | Purged with the record, because `experience_grants` has no foreign key to it and a re-appearing ID would otherwise re-apply them | +| 7 | `lifecycle_events` | The record's own history | +| 8 | `experience_embeddings` | Guarded by `to_regclass`: the table belongs to the vectors package, and a text-only deployment simply skips the step | +| 9 | `experience_records` | The tombstone, last, so every scope-dependent sweep above still had its scope | + +**`experience_grant_access` rows are deliberately kept.** They name a grant and a principal, carry no record +payload, and are the answer to "who read this before it was deleted" — which is exactly the question a deletion +makes urgent. + +**Authorization is decided once, at step 1, and every step below it follows the record's ID rather than the +caller's scope.** That is not an oversight, and it is true of *all* of steps 2–8, not only the ones without scope +columns: + +- `confidence_evidence` and `reuse_feedback_exposures` carry no scope columns at all (by design — see `0007` and + `0008`), so "every row that named this record" is the only thing an erasure could mean for them. +- `experience_grants`, `experience_grant_events` and `experience_embeddings` *do* carry the six owner-scope + columns, copied from the record row when they were written, and are still matched on the record's ID alone. + That is a choice. In practice the predicates coincide, because every grant and every vector this library writes + copies its scope from the record; a row that disagrees was written outside this library, over an ID whose + content is now gone, and is exactly the row nothing else would ever collect. + +One consequence is worth stating plainly: if another scope recorded feedback naming this record's ID — which +`0008` deliberately allows, because a run that saw an ID resolving to nothing must still be recordable — that +exposure row goes with the erasure, and its submission goes too if this record was the only one it named. Erasing +a record's traces is what was asked for; it just is not confined to the scope that asked. + +**Two purges sharing one feedback submission cannot orphan it.** A submission may name several records; erasing +two of them at once used to leave the parent row behind with zero exposures, because each purge's "are there any +exposures left?" still saw the other's uncommitted delete. Step 3 now locks the submissions `FOR UPDATE`, in +`feedback_id` order, *before* deleting any exposure, so the second purge asks its question after the first has +committed. That row carries a run ID, a scope, an outcome, a measure and — for a human assessment — a reviewer +identity and a free-text rationale, so an orphan is not a tidiness problem. + +### A tombstone is terminal + +| A late… | Answer | Enforced by | +| --- | --- | --- | +| `CreateAsync` under the erased ID | `Conflict`, as for any taken ID, revealing nothing about which scope holds it | **Schema** — the primary key collides with the surviving tombstone row | +| `CommitLifecycleEventAsync` | `Deleted`. Nothing is appended | **Both** — the adapter's predicate, and `0010`'s projection guard, which refuses any `UPDATE` of a tombstone from the database's own side | +| confidence submission | `Deleted`, with no ledger row: an erased record's ID must not go back into a table the erasure emptied | Adapter | +| reuse-feedback write naming it | `Invalid`, naming the exposure by position. Only tombstones in the submission's own scope are visible to that check | Adapter | +| embedding write | `Missing`, never `Stale`: no revision of an erased record can ever be indexed | Adapter | +| grant over it | `NotFound`: there is nothing left to share | Adapter | +| any `UPDATE` of the tombstone row, marker or not | Refused, `42501` | **Schema** | +| any `DELETE` or `TRUNCATE` of the record row, marker or not | Refused, `42501` | **Schema** | +| `GetAsync` / `GetHistoryAsync` in the owning scope | `Deleted`, with no record and no events | Adapter | +| `QueryAsync`, text search, vector search | The tombstone is simply absent | Adapter | +| anything at all from another scope | `NotFound`, exactly as for an ID that never existed | Adapter | + +**The distinction in that last column matters, so read it rather than the summary.** Four of the write refusals +are *adapter*-enforced: they are predicates this library puts in its own statements, and raw SQL from another +tool can still `INSERT` a lifecycle event, a confidence-evidence row, an exposure, an embedding or a grant +against a tombstoned ID. None of those tables has a foreign key to `experience_records`, deliberately (`0002`, +`0005`, `0007`, `0008`), and adding one now would rewrite four journaled tables' shapes for this one rule. What +*is* schema-enforced is the part that cannot be worked around: the ID can never be re-created, the tombstone can +never be moved, and the record row can never be removed — which together mean an ID, once spent, is spent. + +**Those adapter predicates are locked, not merely read.** Every write that gates on "this record is not a +tombstone" takes `FOR KEY SHARE` on the record row in the same statement, so a writer that started before an +erasure committed is parked against the purge's own `FOR UPDATE` and re-checks when it is released, instead of +deciding against a snapshot the purge has already invalidated. Without that, a write issued a moment after a +`DeleteAsync` returned `Deleted` could still land: a stored vector derived from the erased summary and lesson, a +live 90-day grant over a spent ID, or a reviewer's identity and free-text rationale about the erased record, +permanently, in an append-only table. `FOR KEY SHARE` rather than `FOR SHARE` on purpose — it is the weakest mode +that still conflicts with the purge, and it does not block an ordinary lifecycle commit. + +### Retention + +There is **no default retention and no timer**. Nothing expires unless a host asks for it, and this library ships no +scheduler, no background service and no hosted service: when a sweep runs is the host's decision, because only the +host knows its obligations. + +```csharp +// Erase this scope's records older than 90 days, at most 200 at a time. +var sweep = await store.SweepExpiredAsync(hostAuthorization, scope, TimeSpan.FromDays(90), batchSize: 200, cancellationToken); +// sweep.DeletedCount, and sweep.MoreRemain when another pass would find more. + +// Expired sharing grants, with their audit events. Administrator authority, like every other grant mutation. +var grants = new PostgresExperienceGrantStore(dataSource); +var purged = await grants.PurgeExpiredAsync(hostAuthorization, administration, scope, batchSize: 200, cancellationToken); +``` + +Age is measured from `CreatedAt` on the store's own `TimeProvider`, never from `UpdatedAt`: age is how long this +library has held the data, and a record that is read, ranked, or re-scored does not thereby become younger. Each +record in a batch is erased in its own transaction, so an interrupted sweep leaves every record it reached wholly +erased and every record it did not reach wholly untouched. A non-positive age is `Invalid` — there is no retention +age that means "delete everything" — and so is a batch outside 1…500. + +#### A sweep reaches one scope, exactly, and says nothing about the scopes beneath it + +This is the one operation here whose failure mode is **a missed retention obligation reported as success**, so it +gets its own heading rather than a clause. + +`SweepExpiredAsync` matches `scope` field for field, exactly as every other operation in this library matches it. +A sweep of `new Scope(tenant, app, project)` — team, agent and user all null — reaches only the records stored +with all three of those fields null. Every record the same tenant holds under a team, an agent or a user is a +**different scope**: it is not swept, it is not counted, and the call comes back +`Outcome: Deleted, DeletedCount: 0, MoreRemain: false` — which reads exactly like "there was nothing to delete". + +```csharp +// WRONG, if this tenant ever wrote records under a team, an agent, or a user. +await store.SweepExpiredAsync(auth, new Scope(tenant, app, project), TimeSpan.FromDays(90), 200, ct); + +// Right: the host enumerates every leaf scope it has written under, and sweeps each one. +foreach (var leaf in hostOwnedScopes) // only the host knows which of these exist +{ + var sweep = await store.SweepExpiredAsync(auth, leaf, TimeSpan.FromDays(90), 200, ct); + while (sweep.MoreRemain) { sweep = await store.SweepExpiredAsync(auth, leaf, TimeSpan.FromDays(90), 200, ct); } +} +``` + +The library cannot enumerate those scopes for you. A scope is the host's own partitioning; nothing here knows +which team, agent or user values exist, and inventing a prefix match would silently widen a *destructive* +operation, which is the one direction this library never widens anything. A host with a tenant-wide retention +policy has to keep its own list of the leaf scopes it writes under, and a host that cannot must not read +`MoreRemain: false` as "this tenant is clean". + +#### Stopping early + +Erasure is the one thing this library cannot undo, so a sweep that stops half-way never throws away how much it +destroyed: + +- **Cancellation** between records *returns* the partial result, with `Interrupted: true` and `MoreRemain: true`. + Cancelling a sweep is a normal way to run one, and a host that asked for it still needs the count for its own + compliance log. +- **A storage failure** part-way throws `ExperienceRetentionSweepInterruptedException`, which carries the same + partial result on `.Partial` and is an `ExperienceStoreException` like every other storage failure here — so a + host that already catches those keeps working and does not have to learn a new type to stay correct. + +`PurgeExpiredAsync` collects a grant once its stored `expires_at` has passed (and any grant naming a record that is +already a tombstone). A revoked grant that has **not** expired is left alone: its revocation is a fact about a +window that is still open, and it is collected when that window closes. The cutoff is +`LEAST(hostClock, clock_timestamp())`: everywhere a grant is *read*, this schema deliberately uses the database's +clock so a host whose clock is wrong cannot widen a permission, and this is the one grant operation that +*destroys* rows — a host skewed a day forward must not be able to erase grants the database still considers live. +The batch bound is applied inside the function too, not only by the validator, because `LIMIT NULL` means "no +limit" in PostgreSQL and a hand-caller passing `NULL` would otherwise get an unbounded destructive sweep. + +### The honesty statement, and the limits + +Erasure needs `DELETE` on five append-only tables. `0006` documented a manual runbook for that — +`ALTER TABLE … DISABLE TRIGGER`, delete, re-enable — and `0010` replaces it rather than automating it: the guards +themselves recognise one transaction-scoped marker, `SET LOCAL agent_experience.purge_authorized = 'on'`, set only +inside the purge function, and they go on refusing `UPDATE` and `TRUNCATE` unconditionally in every session, +including the purging one. Nothing is ever disabled, and no other connection's window is widened for an instant. + +**This is an auditability mechanism, not a privilege boundary, and it must not be read as one.** + +- A custom GUC is settable by any session. Nothing stops a connection that already has `DELETE` on these tables + from issuing the same `SET LOCAL` itself and then deleting from them directly. The marker decides whether a + *permitted* delete is refused; it is not what decides permission. +- The guards still do not bind a role that can `ALTER TABLE` — which is the application role, because it created + the tables. An owner can disable or drop a trigger and write what it likes. + +What the purge path actually buys is narrower and real: erasure has exactly **one** code path, inside **one** +transaction, with the guard never switched off, never left off across a failure, and never visible to another +session. It is a guard against a bug, a careless script, or a compromised application path — not against an +administrator who has decided to tamper. A deployment that needs more must own these tables with a role the +application does not have. + +**There is exactly one real privilege boundary here, and `0010` creates it.** Both purge functions are +`SECURITY DEFINER`, and PostgreSQL grants `EXECUTE` on a new function to `PUBLIC` by default — which would make +them a universally callable erasure primitive, reachable by any role that can connect, over any tenant whose +`experience_id` and scope it can `SELECT`. `0010` therefore revokes `EXECUTE` from `PUBLIC` and grants it back +only to the role that applied the migration, which owns these tables and is the role the application runs as. A +deployment whose application role is *not* the migrating role must grant it once, explicitly, and to nothing +else: + +```sql +GRANT EXECUTE ON FUNCTION agent_experience.purge_experience_record( + uuid, text, text, text, text, text, text, bigint, timestamptz) TO ; +GRANT EXECUTE ON FUNCTION agent_experience.purge_expired_grants( + text, text, text, text, text, text, timestamptz, integer) TO ; +``` + +**A record whose run was erased can never be finalized again.** `ExperienceFinalizationService.ExperienceIdFor` +derives a record's ID from the run *and the scope*, deterministically, so replaying finalization for that run +derives the same ID, collides with the tombstone, and stops — permanently. That is the intended terminal +semantics: re-finalizing would recreate exactly what the deletion removed. It is the same *shape* of permanent +dead end that mixing the scope into the derivation just closed, and the difference is what matters — the old one +was reachable from any scope and undiagnosable, because `CreateAsync`'s conflict is deliberately scope-blind, +while this one is reachable only by the scope that owns the record and that scope can see exactly why: +`GetAsync` answers `Deleted` for its own tombstone. (That change removed the one-argument +`ExperienceIdFor(Guid)` with no compatible overload. The library is pre-1.0 and unpublished, and an `[Obsolete]` +overload could not have been kept honestly — it would have to go on deriving the squattable ID. Callers pass the +same `Scope` they finalize under; nothing persisted needs migrating, because a record's ID is stored, never +re-derived.) + +**What deletion does not reach**, stated rather than buried: + +- **Backups, replicas, WAL, and logical-replication streams.** Host-owned, and out of reach of this schema. A + deployment with a retention obligation has to reach them itself. +- **Exported telemetry.** Spans and metrics this library emitted carry record IDs; erasing a record does not + retract them. +- **External artifacts a record merely named.** Tickets, logs, commits: the library never held them. +- **The dead heap tuple — until `VACUUM`, the erased text is still in this database.** The tombstone is written + with an `UPDATE`, and an `UPDATE` in PostgreSQL writes a new row version and leaves the old one in the heap. + Until `VACUUM` reclaims it, the previous version of the record row still carries the task summary, the lesson, + the attempt results and the task ID, readable by anyone who can inspect the page — `pageinspect`, a file-level + copy, a base backup taken in that window. The same is true of every row the erasure deleted. Autovacuum will + get there on its own schedule, which is not a schedule anybody promised; an obligation with a deadline has to + run `VACUUM agent_experience.experience_records` (and the other swept tables) itself. `VACUUM` does not + overwrite the freed bytes either, so defeating forensic recovery of freed pages needs `VACUUM FULL` — which + rewrites the table under an `ACCESS EXCLUSIVE` lock — or a storage-level guarantee. +- **Dead index entries.** Entries in the GIN index over `search_vector`, and in the out-of-band HNSW index over + `experience_embeddings`, persist until `VACUUM` reclaims them. *These* really do point at row versions that no + longer carry the erased text, so they cannot return it — the distinction from the bullet above is exact, and + was worth stating both ways round. + ## Schema The schema lives in the embedded scripts under `Migrations/`. @@ -831,8 +1098,52 @@ its triggers as to `0006`'s: read them above before relying on them. The ceiling above is relative to `issued_at`, so without it a bypassing writer could store an effectively permanent grant simply by dating it a century forward; a `CHECK` cannot say this, because it may not call `now()`. -- Retention is deferred to roadmap story 4.5 with the other ledgers'. This is the one most likely to grow without - bound in a deployment that shares heavily, so plan it before enabling auditing at scale. +- Retention: this ledger is deliberately **not** swept by a record erasure, because who read a record before it was + deleted outlives the record. It is the table most likely to grow without bound in a deployment that shares + heavily, so plan its retention — which is the host's, on host-owned terms — before enabling auditing at scale. + +`0010_delete_and_expire.sql` adds the one erasure path (see [Deleting and expiring data](#deleting-and-expiring-data)): + +- `experience_records.deleted_at`, nullable, so no existing row is rewritten, plus + `experience_records_tombstone_shape` — added `ALTER TABLE … NOT VALID` like every other `CHECK` on an existing + table — which makes "erased" one shape rather than a flag a writer could set over a payload that is still there. +- `agent_experience.purge_experience_record`, a `SECURITY DEFINER` function holding the whole erasure: the scope + and revision guards, the seven tables it sweeps in the order above, and the tombstone. + `agent_experience.purge_expired_grants` does the same for expired grants and their events. +- `agent_experience.purge_authorized()`, which reads the transaction-scoped marker the guards recognise, and + replacements for `0006`'s `reject_event_log_mutation` and `reject_audited_grant_delete` and `0007`'s + `enforce_record_projection`. They are replaced with `CREATE OR REPLACE`, so every `ENABLE ALWAYS` binding + survives and no table is unguarded for an instant; nothing is dropped, disabled, or recreated. `UPDATE` and + `TRUNCATE` stay refused unconditionally, and `DELETE` is admitted only under the marker and only on the five + tables an erasure sweeps — `experience_grant_access` is deliberately not one of them. +- `agent_experience.reject_record_removal`, and the only two triggers this script creates: + `experience_records_no_delete` and `experience_records_no_truncate`, both `ENABLE ALWAYS`. A bare + `DELETE FROM agent_experience.experience_records` used to succeed from any session with `DELETE` on the table — + orphaning the whole audit trail, none of which has a foreign key back to the record, and **freeing the ID**, so + that a record re-created under it inherits every grant issued over the old content. It is refused now with no + marker clause and no exception at all, because the erasure never deletes that row: it updates it into a + tombstone, which is the point. +- The projection guard gains two rules: a tombstone can never be updated again, by anyone, and `deleted_at` can be + set only inside the purge. The marked exception is shape-checked rather than merely marker-checked — a live row, + to an empty-payload tombstone, in its own scope, with its own `payload_version` and a `created_at` equal to the + deletion instant, one revision forward — so a marked transaction may make that one transition and no other. The + scope columns are part of that check for a concrete reason: without them one marked `UPDATE` could tombstone a + record *into another tenant's scope*, leaving the owning scope seeing `NotFound` for its own erased record. +- `ix_experience_records_live_by_age`, partial on `deleted_at IS NULL`, which is the retention sweep's whole + predicate; plus the `confidence_evidence (experience_id)` and `reuse_feedback_exposures (experience_id)` indexes + `0007` and `0008` each deferred to this story, because this is the query that justifies them. **All three are + built with plain `CREATE INDEX` inside the migrator's per-script transaction**, which takes a `SHARE` lock and + blocks writes to those tables for the duration — and one of them is over `experience_records`, the table this + library writes most, so on an established database this is a larger write outage than `0007`'s, `0008`'s or + `0009`'s. `CREATE INDEX CONCURRENTLY` cannot run in a transaction block at all, so it cannot simply be swapped; + the script's header carries the out-of-band runbook (add the column, build all three `CONCURRENTLY`, then + migrate, at which point `IF NOT EXISTS` makes the script's own statements no-ops), exactly as `0007`, `0008` + and `0009` do for theirs. +- `REVOKE ALL … FROM PUBLIC` on both purge functions, and `GRANT EXECUTE … TO CURRENT_USER`. Without it, + PostgreSQL's default `EXECUTE`-to-`PUBLIC` on a `SECURITY DEFINER` function would make erasure available to + every role that can connect. +- The script's header carries the honesty statement, the retained list (including `payload_version`), the + privilege note, the `CONCURRENTLY` runbook, the dead-heap-tuple limit, and the confirm-then-`VALIDATE` step. **This package's schema stops there, and that is deliberate.** The derived embedding schema — the `vector` extension and the `experience_embeddings` table — belongs to the companion package @@ -871,7 +1182,13 @@ var migration = await ExperienceSchemaMigrator.MigrateAsync(dataSource, cancella exact-scope predicate (see [Sharing grants](#sharing-grants)). Administering grants additionally needs `INSERT` and `UPDATE` on `agent_experience.experience_grants` and `INSERT` on `agent_experience.experience_grant_events`. Recording reuse feedback needs `SELECT` and `INSERT` on `agent_experience.reuse_feedback` and - `agent_experience.reuse_feedback_exposures`, and ownership of both to create `0008`'s triggers. + `agent_experience.reuse_feedback_exposures`, and ownership of both to create `0008`'s triggers. Deleting needs + `EXECUTE` on `agent_experience.purge_experience_record` and `agent_experience.purge_expired_grants` — `0010` + revokes both from `PUBLIC` and grants them to the migrating role only, so an application role that is *not* the + migrating role has to be granted `EXECUTE` explicitly (see + [Deleting and expiring data](#the-honesty-statement-and-the-limits)) and nothing else should be. The migrating + role also has to own the tables whose guard functions `0010` replaces, and `experience_records` itself, to + create `0010`'s two removal-guard triggers on it — which it does when it created them. - **Connections.** The data source must allow at least two concurrent connections: one for the advisory lock and one for the scripts. A multiplexing data source (`NpgsqlDataSourceBuilder.EnableMultiplexing`) cannot hold a session advisory lock, because its commands do not stay on one physical connection, so it is not supported for migration. @@ -905,7 +1222,11 @@ definition, and a rename would reapply it. Change the schema by adding the next- ## Data semantics - **One write path per change.** Each create is a single `INSERT`. The only update is a lifecycle commit, which is - always paired with its event in one transaction (see above). Nothing deletes a record or an event. + always paired with its event in one transaction (see above). The only deletion is `DeleteAsync` and the retention + sweep that runs it, which erase payload and leave a tombstone (see + [Deleting and expiring data](#deleting-and-expiring-data)); no other path *in this library* removes a record, an + event, or a ledger row, and for the record row itself `0010`'s removal guard makes that true of the schema + rather than only of the library — a bare `DELETE` or `TRUNCATE` is refused from every session, marker or not. - **UTC timestamps.** Every timestamp is stored and returned in UTC. `CreatedAt`, `UpdatedAt`, and a lifecycle event's `OccurredAt` are columns, and PostgreSQL keeps microsecond precision, so sub-microsecond ticks are truncated on write. Nested timestamps are stored in the payload at full precision. diff --git a/tests/AgentExperience.Abstractions.Tests/ContractShapeTests.cs b/tests/AgentExperience.Abstractions.Tests/ContractShapeTests.cs index 07ebd79..5732344 100644 --- a/tests/AgentExperience.Abstractions.Tests/ContractShapeTests.cs +++ b/tests/AgentExperience.Abstractions.Tests/ContractShapeTests.cs @@ -491,7 +491,10 @@ public void Eligibility_for_reuse_is_stated_once_and_matches_the_status_doc() public void Store_outcomes_results_and_exception_have_the_expected_shape() { Assert.Equal( - ["Created", "Found", "NotFound", "Denied", "Invalid", "Conflict", "Committed", "StaleRevision", "StatusMismatch", "ReplacementNotAllowed"], + [ + "Created", "Found", "NotFound", "Denied", "Invalid", "Conflict", "Committed", "StaleRevision", + "StatusMismatch", "ReplacementNotAllowed", "Deleted", + ], Enum.GetNames()); var error = new StoreValidationError("Scope.TenantId", "must not be empty or whitespace."); diff --git a/tests/AgentExperience.Core.Tests/Diagnostics/ExperienceLoop.cs b/tests/AgentExperience.Core.Tests/Diagnostics/ExperienceLoop.cs index 66c7706..8ba6672 100644 --- a/tests/AgentExperience.Core.Tests/Diagnostics/ExperienceLoop.cs +++ b/tests/AgentExperience.Core.Tests/Diagnostics/ExperienceLoop.cs @@ -164,7 +164,7 @@ [new RequiredCheck("tests")], Now, cancellationToken); - var experienceId = ExperienceFinalizationService.ExperienceIdFor(runId); + var experienceId = ExperienceFinalizationService.ExperienceIdFor(runId, Scope); // Seeded at the revision the initial lifecycle event will leave the record at, with a summary // that carries the marker: the post-commit indexing hook then really embeds poisoned text. diff --git a/tests/AgentExperience.Core.Tests/Diagnostics/ExperienceTelemetryTests.cs b/tests/AgentExperience.Core.Tests/Diagnostics/ExperienceTelemetryTests.cs index 5dfb6f9..d235903 100644 --- a/tests/AgentExperience.Core.Tests/Diagnostics/ExperienceTelemetryTests.cs +++ b/tests/AgentExperience.Core.Tests/Diagnostics/ExperienceTelemetryTests.cs @@ -737,7 +737,7 @@ await capture.AppendAttemptAsync( new AppendAttemptRequest(Guid.NewGuid(), ExperienceLoop.Now, TimeSpan.FromSeconds(1), [], "done", null)); await capture.CompleteRunAsync(runId, Guid.NewGuid(), RunExecutionStatus.Completed, ExperienceLoop.Now.AddSeconds(3)); - var experienceId = ExperienceFinalizationService.ExperienceIdFor(runId); + var experienceId = ExperienceFinalizationService.ExperienceIdFor(runId, ExperienceLoop.Scope); index.Records[experienceId] = new FakeEmbeddingIndex.Row(1, "a retrieval summary"); var finalized = await finalization.FinalizeAsync(FinalizeRequest(runId), CancellationToken.None); diff --git a/tests/AgentExperience.Core.Tests/ExperienceFinalizationServiceTests.cs b/tests/AgentExperience.Core.Tests/ExperienceFinalizationServiceTests.cs index c328739..538869f 100644 --- a/tests/AgentExperience.Core.Tests/ExperienceFinalizationServiceTests.cs +++ b/tests/AgentExperience.Core.Tests/ExperienceFinalizationServiceTests.cs @@ -364,15 +364,52 @@ public async Task The_record_and_initial_event_ids_derive_from_the_run() var result = await harness.FinalizeAsync(); - Assert.Equal(ExperienceFinalizationService.ExperienceIdFor(harness.RunId), result.ExperienceId); + Assert.Equal(ExperienceFinalizationService.ExperienceIdFor(harness.RunId, TestScope), result.ExperienceId); Assert.Equal(ExperienceFinalizationService.InitialEventIdFor(harness.RunId), result.Event!.EventId); Assert.Equal(ExperienceFinalizationService.ReflectionIdFor(harness.RunId), result.Record!.Reflection!.ReflectionId); // Derivation is stable across calls and distinct per purpose and per run. - Assert.Equal(ExperienceFinalizationService.ExperienceIdFor(harness.RunId), ExperienceFinalizationService.ExperienceIdFor(harness.RunId)); - Assert.NotEqual(ExperienceFinalizationService.ExperienceIdFor(harness.RunId), ExperienceFinalizationService.InitialEventIdFor(harness.RunId)); - Assert.NotEqual(ExperienceFinalizationService.ExperienceIdFor(harness.RunId), ExperienceFinalizationService.ExperienceIdFor(Guid.NewGuid())); - Assert.NotEqual(Guid.Empty, ExperienceFinalizationService.ExperienceIdFor(harness.RunId)); + Assert.Equal( + ExperienceFinalizationService.ExperienceIdFor(harness.RunId, TestScope), + ExperienceFinalizationService.ExperienceIdFor(harness.RunId, TestScope)); + Assert.NotEqual( + ExperienceFinalizationService.ExperienceIdFor(harness.RunId, TestScope), + ExperienceFinalizationService.InitialEventIdFor(harness.RunId)); + Assert.NotEqual( + ExperienceFinalizationService.ExperienceIdFor(harness.RunId, TestScope), + ExperienceFinalizationService.ExperienceIdFor(Guid.NewGuid(), TestScope)); + Assert.NotEqual(Guid.Empty, ExperienceFinalizationService.ExperienceIdFor(harness.RunId, TestScope)); + } + + [Fact] + public void A_derived_record_id_is_distinct_per_scope_so_no_other_scope_can_squat_it() + { + var runId = Guid.NewGuid(); + var mine = ExperienceFinalizationService.ExperienceIdFor(runId, TestScope); + + // One differing field is enough, required or optional, and an absent optional field is not the + // empty string: each of these is a different scope and must derive a different ID. + foreach (var other in new[] + { + new Scope("tenant-9", "app-1", "project-1"), + new Scope("tenant-1", "app-9", "project-1"), + new Scope("tenant-1", "app-1", "project-9"), + new Scope("tenant-1", "app-1", "project-1", TeamId: "team-1"), + new Scope("tenant-1", "app-1", "project-1", AgentId: "agent-1"), + new Scope("tenant-1", "app-1", "project-1", UserId: "user-1"), + new Scope("tenant-1", "app-1", "project-1", TeamId: string.Empty), + }) + { + Assert.NotEqual(mine, ExperienceFinalizationService.ExperienceIdFor(runId, other)); + } + + // Adjacent fields cannot be re-divided into the same byte sequence, which is what the length + // prefixes buy: ("a", "bc") and ("ab", "c") are different scopes. + Assert.NotEqual( + ExperienceFinalizationService.ExperienceIdFor(runId, new Scope("a", "bc", "p")), + ExperienceFinalizationService.ExperienceIdFor(runId, new Scope("ab", "c", "p"))); + + Assert.Throws(() => ExperienceFinalizationService.ExperienceIdFor(runId, null!)); } [Fact] @@ -398,20 +435,32 @@ public async Task A_retry_after_a_failed_initial_commit_finishes_that_commit_rat } [Fact] - public async Task A_derived_record_id_taken_in_another_scope_is_a_failure_not_a_silent_success() + public async Task Another_scope_cannot_block_a_run_by_taking_the_id_it_will_finalize_under() { var harness = await Harness.WithCompletedRunAsync(); + + // The squat this test used to pin: a foreign scope writing a record under the ID the run was + // going to finalize under, which left the run permanently unable to finalize and -- because + // CreateAsync's conflict is deliberately scope-blind -- with no way to find out why. The ID is + // now derived from the scope as well as the run, so a writer in another scope does not have it: + // the ID it can derive from this run is a different one, and taking that one blocks nothing. var foreignScope = new Scope("tenant-9", "app-1", "project-1"); - harness.Store.Seed(TestRecord(ExperienceFinalizationService.ExperienceIdFor(harness.RunId), foreignScope)); + harness.Store.Seed(TestRecord( + ExperienceFinalizationService.ExperienceIdFor(harness.RunId, foreignScope), + foreignScope)); var result = await harness.FinalizeAsync(); - Assert.Equal(FinalizationOutcome.Failed, result.Outcome); - Assert.Equal(FinalizationStage.CreateRecord, result.Stage); - Assert.False(result.IsDurable); - Assert.Empty(harness.Store.Commits); + Assert.Equal(FinalizationOutcome.Validated, result.Outcome); + Assert.True(result.IsDurable); + Assert.Equal(ExperienceFinalizationService.ExperienceIdFor(harness.RunId, TestScope), result.ExperienceId); + Assert.Single(harness.Store.Commits); } + // A derived ID already taken *inside* this scope is not a squat and is not a failure: it is this + // run's own earlier attempt, which finalization resumes rather than starting over. That path is + // pinned by A_retry_after_a_failed_initial_commit_finishes_that_commit_rather_than_starting_over. + // --------------------------------------------------------------------------------------------- // Store failures // --------------------------------------------------------------------------------------------- @@ -525,7 +574,7 @@ public async Task A_record_finalized_concurrently_converges_on_AlreadyFinalized_ harness.Store.BeforeCommit = store => { store.BeforeCommit = null; - store.ForceFinalize(ExperienceFinalizationService.ExperienceIdFor(harness.RunId), ExperienceStatus.Validated); + store.ForceFinalize(ExperienceFinalizationService.ExperienceIdFor(harness.RunId, TestScope), ExperienceStatus.Validated); }; var result = await harness.FinalizeAsync(); diff --git a/tests/AgentExperience.MicrosoftAgentFramework.Tests/ExperienceFinalizationWiringTests.cs b/tests/AgentExperience.MicrosoftAgentFramework.Tests/ExperienceFinalizationWiringTests.cs index 609f505..1f0acab 100644 --- a/tests/AgentExperience.MicrosoftAgentFramework.Tests/ExperienceFinalizationWiringTests.cs +++ b/tests/AgentExperience.MicrosoftAgentFramework.Tests/ExperienceFinalizationWiringTests.cs @@ -63,7 +63,7 @@ public async Task A_completed_run_is_finalized_into_a_durable_Validated_record() // The record is the one this invocation's run produced. var runId = Assert.Single(harness.Service.StartedRunIds); - Assert.Equal(ExperienceFinalizationService.ExperienceIdFor(runId), result.ExperienceId); + Assert.Equal(ExperienceFinalizationService.ExperienceIdFor(runId, TestScope), result.ExperienceId); Assert.Equal(runId, result.Record!.SourceRunId); Assert.Equal(ExperienceStatus.Validated, harness.Store.StatusOf(result.ExperienceId!.Value)); } diff --git a/tests/AgentExperience.ReuseBaseline/GoldenFailedTrialReport.txt b/tests/AgentExperience.ReuseBaseline/GoldenFailedTrialReport.txt index be18643..700c758 100644 --- a/tests/AgentExperience.ReuseBaseline/GoldenFailedTrialReport.txt +++ b/tests/AgentExperience.ReuseBaseline/GoldenFailedTrialReport.txt @@ -192,10 +192,10 @@ instead of taking it on trust. come out that way is a property of this table, not a finding. LEARNED RECORDS -- produced by the learning phase, read back out of the store: - learn-settlement-batch-stalled experience dbc59b8d-ddfe-8df0-8858-beb0df9f1484 + learn-settlement-batch-stalled experience 05cfee0f-f21f-853d-beaf-0f5c49c0e2f1 ExperienceStatus.Validated, reuse confidence 0.667, 2 failed attempt(s) in the learning run working approach in the lesson: 'wait-for-lock' (read out of the run's final successful attempt, not from the task set) - learn-settlement-gateway-shedding experience 80eeab78-7f83-8d3c-a6c8-e2577f833e34 + learn-settlement-gateway-shedding experience 3308130e-1e7a-8c8d-a04b-ae736bfb0828 ExperienceStatus.Validated, reuse confidence 0.667, 3 failed attempt(s) in the learning run working approach in the lesson: 'escalate-to-oncall' (read out of the run's final successful attempt, not from the task set) diff --git a/tests/AgentExperience.ReuseBaseline/GoldenNegativeControlReport.txt b/tests/AgentExperience.ReuseBaseline/GoldenNegativeControlReport.txt index fe78d1f..3311b5b 100644 --- a/tests/AgentExperience.ReuseBaseline/GoldenNegativeControlReport.txt +++ b/tests/AgentExperience.ReuseBaseline/GoldenNegativeControlReport.txt @@ -192,10 +192,10 @@ instead of taking it on trust. come out that way is a property of this table, not a finding. LEARNED RECORDS -- produced by the learning phase, read back out of the store: - learn-settlement-batch-connector-flap experience e6796bc8-8d90-8e66-91fd-0ef572f9b922 + learn-settlement-batch-connector-flap experience 5bdcb457-859f-8c23-b438-9c338d01a0b1 ExperienceStatus.Validated, reuse confidence 0.667, 0 failed attempt(s) in the learning run working approach in the lesson: 'retry-immediately' (read out of the run's final successful attempt, not from the task set) - learn-settlement-gateway-projection-drift experience c91e18b8-7ac3-87dd-9e16-14d3661f4396 + learn-settlement-gateway-projection-drift experience 148e9569-f6c8-8e02-b778-b8f3bd4325be ExperienceStatus.Validated, reuse confidence 0.667, 1 failed attempt(s) in the learning run working approach in the lesson: 'rebuild-index' (read out of the run's final successful attempt, not from the task set) diff --git a/tests/AgentExperience.ReuseBaseline/GoldenReport.txt b/tests/AgentExperience.ReuseBaseline/GoldenReport.txt index 6b97225..15f17c4 100644 --- a/tests/AgentExperience.ReuseBaseline/GoldenReport.txt +++ b/tests/AgentExperience.ReuseBaseline/GoldenReport.txt @@ -192,10 +192,10 @@ instead of taking it on trust. come out that way is a property of this table, not a finding. LEARNED RECORDS -- produced by the learning phase, read back out of the store: - learn-settlement-batch-stalled experience dbc59b8d-ddfe-8df0-8858-beb0df9f1484 + learn-settlement-batch-stalled experience 05cfee0f-f21f-853d-beaf-0f5c49c0e2f1 ExperienceStatus.Validated, reuse confidence 0.667, 2 failed attempt(s) in the learning run working approach in the lesson: 'wait-for-lock' (read out of the run's final successful attempt, not from the task set) - learn-settlement-gateway-shedding experience 80eeab78-7f83-8d3c-a6c8-e2577f833e34 + learn-settlement-gateway-shedding experience 3308130e-1e7a-8c8d-a04b-ae736bfb0828 ExperienceStatus.Validated, reuse confidence 0.667, 3 failed attempt(s) in the learning run working approach in the lesson: 'escalate-to-oncall' (read out of the run's final successful attempt, not from the task set) diff --git a/tests/AgentExperience.Sample.EndToEnd.Tests/GoldenTranscript.txt b/tests/AgentExperience.Sample.EndToEnd.Tests/GoldenTranscript.txt index 3ebd32e..2346940 100644 --- a/tests/AgentExperience.Sample.EndToEnd.Tests/GoldenTranscript.txt +++ b/tests/AgentExperience.Sample.EndToEnd.Tests/GoldenTranscript.txt @@ -31,7 +31,7 @@ clock: a stepping fixture; identifiers: a counter. Two runs print the same byte [4] finalize The verified run is reflected on and persisted as one Experience Record. -> FinalizationOutcome.Validated -> IsDurable=true - experience 219b3e10-e601-833a-bd12-e137df161b37, ExperienceStatus.Validated, revision 1 (read back from the store) + experience 0daf4d74-d8a8-8597-a512-ae7a6c3f1068, ExperienceStatus.Validated, revision 1 (read back from the store) reuse confidence 0.667, 2 attempts kept: #0 failed, #1 succeeded lesson: Task 'refund-ticket-triage' verified: required checks [refund-check] passed (evidence: 44444444-4444-4444-8444-444444444444). failed approaches recorded: 1; successful: 1 @@ -40,12 +40,12 @@ clock: a stepping fixture; identifiers: a counter. Two runs print the same byte -> RetrievalOutcome.Completed -> candidates ranked: 1 eligible statuses: Validated, Reinforced; confidence floor 0.50 - top candidate: experience 219b3e10-e601-833a-bd12-e137df161b37, ExperienceStatus.Validated + top candidate: experience 0daf4d74-d8a8-8597-a512-ae7a6c3f1068, ExperienceStatus.Validated text-only retrieval: no vector channel is configured, which is a supported deployment [6] inject The record is injected into run B as a labelled Historical Reference. -> InjectionOutcome.Injected - -> injected: 219b3e10-e601-833a-bd12-e137df161b37 + -> injected: 0daf4d74-d8a8-8597-a512-ae7a6c3f1068 byte budget used: 2059 of 16384; record limit 8 omitted: 0; excluded before ranking: 0; truncated search: false read out of the 2059 bytes run B's model was handed: lesson present, source named, confidence named, applicability named, raw captured result absent @@ -55,7 +55,7 @@ clock: a stepping fixture; identifiers: a counter. Two runs print the same byte -> ExperienceReuseFeedbackOutcome.Recorded -> Outcome: Recorded | Benefit: Unknown | nothing moved feedback 55555555-5555-4555-8555-555555555555 against run ae000000-0000-4000-8000-000000000007 - ReuseAttributionSource.None; experience 219b3e10-e601-833a-bd12-e137df161b37: ExperienceExposureDisposition.ExposureOnly, counted=false + ReuseAttributionSource.None; experience 0daf4d74-d8a8-8597-a512-ae7a6c3f1068: ExperienceExposureDisposition.ExposureOnly, counted=false exposure is not attribution: the run saw the record and the run ended, and nothing in those two facts attributes one to the other. Moving a confidence score needs a human assessment or a comparative evaluation, and the sample performed neither, so it submits neither. diff --git a/tests/AgentExperience.Storage.Postgres.Tests/OfflineStoreTests.cs b/tests/AgentExperience.Storage.Postgres.Tests/OfflineStoreTests.cs index 678c699..783bb6d 100644 --- a/tests/AgentExperience.Storage.Postgres.Tests/OfflineStoreTests.cs +++ b/tests/AgentExperience.Storage.Postgres.Tests/OfflineStoreTests.cs @@ -378,6 +378,7 @@ public void Embedded_schema_script_is_available_and_creates_the_versioned_table( PostgresExperienceRecordSchema.ConfidenceEvidenceScriptName, PostgresExperienceRecordSchema.ReuseFeedbackScriptName, PostgresExperienceRecordSchema.GrantAccessLogScriptName, + PostgresExperienceRecordSchema.DeleteAndExpireScriptName, ], PostgresExperienceRecordSchema.ScriptNames); Assert.Contains("CREATE SCHEMA IF NOT EXISTS agent_experience", sql, StringComparison.Ordinal); @@ -597,7 +598,7 @@ public void Append_only_script_adds_the_replacement_column_and_the_triggers_that // 0006 is applied after 0005 and before 0007, which the migrator relies on for ordinal name ordering. Assert.Equal( PostgresExperienceRecordSchema.SupersessionAndAppendOnlyScriptName, - PostgresExperienceRecordSchema.ScriptNames[^4]); + PostgresExperienceRecordSchema.ScriptNames[^5]); Assert.Equal( PostgresExperienceRecordSchema.ScriptNames.Order(StringComparer.Ordinal), PostgresExperienceRecordSchema.ScriptNames); @@ -659,7 +660,7 @@ public void Confidence_script_adds_the_evidence_ledger_and_guards_the_columns_it // 0007 is applied after 0006 and before 0008, which the migrator relies on for ordinal name ordering. Assert.Equal( PostgresExperienceRecordSchema.ConfidenceEvidenceScriptName, - PostgresExperienceRecordSchema.ScriptNames[^3]); + PostgresExperienceRecordSchema.ScriptNames[^4]); } [Fact] @@ -737,12 +738,252 @@ public void Reuse_feedback_script_creates_an_append_only_ledger_that_cannot_clai // 0008 is applied after 0007 and before 0009, which the migrator relies on for ordinal name ordering. Assert.Equal( PostgresExperienceRecordSchema.ReuseFeedbackScriptName, - PostgresExperienceRecordSchema.ScriptNames[^2]); + PostgresExperienceRecordSchema.ScriptNames[^3]); + Assert.Equal( + PostgresExperienceRecordSchema.ScriptNames.Order(StringComparer.Ordinal), + PostgresExperienceRecordSchema.ScriptNames); + } + + [Fact] + public void Delete_script_adds_one_erasure_path_and_leaves_every_guard_armed() + { + var script = PostgresExperienceRecordSchema.GetScript(PostgresExperienceRecordSchema.DeleteAndExpireScriptName); + + // The tombstone column, and the CHECK that makes "erased" one shape rather than a flag a writer + // could set over a payload that is still there. Deferred like every CHECK on an existing table, + // with the documented confirm-then-VALIDATE step. + Assert.Contains("ADD COLUMN IF NOT EXISTS deleted_at timestamptz NULL", script, StringComparison.Ordinal); + Assert.Equal(1, CountOccurrences(script, "NOT VALID;")); + Assert.Contains("VALIDATE CONSTRAINT experience_records_tombstone_shape", script, StringComparison.Ordinal); + + // The retained list is stated in the script itself, not only in the README. + Assert.Contains("WHAT IS RETAINED AFTER A DELETE, EXHAUSTIVELY", script, StringComparison.Ordinal); + + // The honesty statement 0006's header demands of anything that touches these guards: this is a + // single code path, not a privilege boundary, and the script has to say so in as many words. + Assert.Contains("NOT A PRIVILEGE BOUNDARY", script, StringComparison.Ordinal); + Assert.Contains("settable by any session", script, StringComparison.Ordinal); + Assert.Contains("ALTER TABLE", script, StringComparison.Ordinal); + + // And what erasure does not reach, stated rather than left to be assumed. + Assert.Contains("WHAT DELETION DOES NOT REACH", script, StringComparison.Ordinal); + Assert.Contains("VACUUM", script, StringComparison.Ordinal); + + var statements = string.Join( + '\n', + script.Split('\n').Where(line => !line.TrimStart().StartsWith("--", StringComparison.Ordinal))); + + // 0006 is journaled and must never be edited, so its guards are replaced in place: every + // ENABLE ALWAYS binding survives, and no trigger is ever dropped, disabled, or recreated through + // a window in which a log would be unguarded. Nothing this script ships is dropped either. + foreach (var destructive in new[] { "DROP TRIGGER", "DROP FUNCTION", "DROP TABLE", "DROP INDEX", "DROP CONSTRAINT" }) + { + Assert.DoesNotContain(destructive, statements, StringComparison.OrdinalIgnoreCase); + } + + Assert.DoesNotContain("DISABLE TRIGGER", statements, StringComparison.OrdinalIgnoreCase); + Assert.DoesNotContain("ALTER COLUMN", statements, StringComparison.OrdinalIgnoreCase); + Assert.DoesNotContain("CREATE EXTENSION", statements, StringComparison.OrdinalIgnoreCase); + + // The only triggers this script creates are the two new ones over experience_records -- the + // guard that makes "no path removes a record row" a property of the schema. Every trigger 0006 + // created is left exactly where it is, guarded by the function bodies replaced above. + var createdTriggers = statements + .Split('\n') + .Select(line => line.Trim()) + .Where(line => line.StartsWith("CREATE TRIGGER ", StringComparison.Ordinal)) + .Select(line => line["CREATE TRIGGER ".Length..]) + .ToArray(); + + Assert.Equal(["experience_records_no_delete", "experience_records_no_truncate"], createdTriggers); + + // ...and both are ENABLE ALWAYS, so they survive session_replication_role = 'replica' exactly as + // 0006's do. A guard a replica connection could step around would not be one. + foreach (var trigger in createdTriggers) + { + Assert.Contains( + $"ALTER TABLE agent_experience.experience_records ENABLE ALWAYS TRIGGER {trigger};", + statements, + StringComparison.Ordinal); + } + + // The bare DELETE the guard closes: the one statement that frees an experience_id, so that a + // record recreated under it inherits every grant issued over the old content. + Assert.Contains("CREATE OR REPLACE FUNCTION agent_experience.reject_record_removal()", statements, StringComparison.Ordinal); + Assert.Contains("BEFORE DELETE ON agent_experience.experience_records", statements, StringComparison.Ordinal); + Assert.Contains("BEFORE TRUNCATE ON agent_experience.experience_records", statements, StringComparison.Ordinal); + + // It takes no marker and has no exception, because the erasure never deletes that row: it + // updates it into a tombstone. A marker clause here would be a bypass with nothing to justify it. + Assert.DoesNotContain("purge_authorized", RejectRecordRemovalBody(statements), StringComparison.Ordinal); + + foreach (var guard in new[] + { + "agent_experience.reject_event_log_mutation", + "agent_experience.reject_audited_grant_delete", + "agent_experience.enforce_record_projection", + }) + { + Assert.Contains($"CREATE OR REPLACE FUNCTION {guard}()", statements, StringComparison.Ordinal); + } + + // The marker is transaction-scoped and set only inside the two purge functions. Anywhere else it + // would be a switch a caller could leave on. + Assert.Equal(2, CountOccurrences(statements, "SET LOCAL agent_experience.purge_authorized = 'on';")); + Assert.Equal(2, CountOccurrences(statements, "SET agent_experience.purge_authorized = 'off'")); + + // The exception is a DELETE on the five tables an erasure sweeps, and nothing else: UPDATE and + // TRUNCATE stay refused in every session, marked or not. + Assert.Contains("IF TG_OP = 'DELETE'", statements, StringComparison.Ordinal); + foreach (var table in new[] + { + "'lifecycle_events'", + "'experience_grant_events'", + "'confidence_evidence'", + "'reuse_feedback'", + "'reuse_feedback_exposures'", + }) + { + Assert.Contains(table, statements, StringComparison.Ordinal); + } + + // Who read a record before it was deleted outlives the record: the access log is not a table the + // marker admits a delete on, and nothing in this script removes a row from it. + Assert.DoesNotContain("'experience_grant_access'", statements, StringComparison.Ordinal); + Assert.DoesNotContain("DELETE FROM agent_experience.experience_grant_access", statements, StringComparison.Ordinal); + + // The erasure order is the frozen one. Evidence before the tombstone, because it has no scope + // columns of its own; children before parents; grant events before grants; the record last. + var order = new[] + { + "DELETE FROM agent_experience.confidence_evidence", + "DELETE FROM agent_experience.reuse_feedback_exposures", + "DELETE FROM agent_experience.reuse_feedback f", + "DELETE FROM agent_experience.experience_grant_events", + "DELETE FROM agent_experience.experience_grants WHERE experience_id", + "DELETE FROM agent_experience.lifecycle_events", + "DELETE FROM agent_experience.experience_embeddings", + "UPDATE agent_experience.experience_records r", + }; + + var previous = -1; + foreach (var step in order) + { + var at = statements.IndexOf(step, StringComparison.Ordinal); + Assert.True(at > previous, $"Erasure step out of order: {step}"); + previous = at; + } + + // The embeddings table belongs to the vectors package, so the base purge tolerates its absence: + // the step is guarded by to_regclass and issued through EXECUTE, which is what keeps a base-only + // database from ever parsing a reference to a table it does not have. + Assert.Contains("to_regclass('agent_experience.experience_embeddings') IS NOT NULL", statements, StringComparison.Ordinal); + Assert.Contains("EXECUTE 'DELETE FROM agent_experience.experience_embeddings", statements, StringComparison.Ordinal); + + // The tombstone writes a fixed value into every column that is not on the retained list. + Assert.Contains("payload = '{}'::jsonb", statements, StringComparison.Ordinal); + Assert.Contains("task_id = '(deleted)'", statements, StringComparison.Ordinal); + Assert.Contains("created_at = p_deleted_at", statements, StringComparison.Ordinal); + Assert.Contains("revision = r.revision + 1", statements, StringComparison.Ordinal); + + // Two submissions sharing one record cannot be left orphaned by two purges racing: the + // submissions are locked before the exposures are deleted, so the second purge's "are there any + // exposures left?" runs after the first has committed rather than against its own stale snapshot. + Assert.True( + statements.IndexOf("FROM agent_experience.reuse_feedback f", StringComparison.Ordinal) + < statements.IndexOf("DELETE FROM agent_experience.reuse_feedback_exposures", StringComparison.Ordinal), + "The shared submissions must be locked FOR UPDATE before their exposures are deleted."); + Assert.Contains("ORDER BY f.feedback_id\n FOR UPDATE;", statements, StringComparison.Ordinal); + + // The marked exception is shape-checked for the scope, the timestamps and the envelope version, + // not only for the payload columns -- otherwise one marked UPDATE could tombstone a record into + // another tenant's scope, where the owner would see NotFound for its own erased record. + foreach (var pinned in new[] + { + "NEW.payload_version = OLD.payload_version", + "NEW.created_at = NEW.deleted_at", + "NEW.updated_at = NEW.deleted_at", + "NEW.tenant_id = OLD.tenant_id", + "NEW.application_id = OLD.application_id", + "NEW.project_id = OLD.project_id", + "NEW.team_id IS NOT DISTINCT FROM OLD.team_id", + "NEW.agent_id IS NOT DISTINCT FROM OLD.agent_id", + "NEW.user_id IS NOT DISTINCT FROM OLD.user_id", + }) + { + Assert.Contains(pinned, statements, StringComparison.Ordinal); + } + + // The grant purge bounds its own batch: LIMIT NULL means "no limit" in PostgreSQL, so a bound + // that lived only in the C# validator was no bound at all for a hand-caller. + Assert.Contains( + $"least(greatest(coalesce(p_limit, {PostgresExperienceRecordStore.MaxSweepBatchSize}), " + + $"{PostgresExperienceRecordStore.MinSweepBatchSize}), {PostgresExperienceRecordStore.MaxSweepBatchSize})", + statements, + StringComparison.Ordinal); + Assert.Contains("LIMIT v_limit", statements, StringComparison.Ordinal); + Assert.DoesNotContain("LIMIT p_limit", statements, StringComparison.Ordinal); + + // ...and it never destroys more than the database itself considers expired, however the host's + // clock is set. Every read of a grant already uses clock_timestamp() for the same reason. + Assert.Contains("least(p_now, pg_catalog.clock_timestamp())", statements, StringComparison.Ordinal); + Assert.DoesNotContain("g.expires_at <= p_now", statements, StringComparison.Ordinal); + + // Step 8's guard checks the embedding table's shape, not only its existence: a divergent table + // would otherwise abort the whole erasure with a bare undefined_column. + Assert.Contains("a.attname = 'experience_id'", statements, StringComparison.Ordinal); + Assert.Contains("ERRCODE = 'undefined_column'", statements, StringComparison.Ordinal); + + // The most severe thing this script could have shipped: two SECURITY DEFINER functions with + // PostgreSQL's default EXECUTE grant to PUBLIC, which would let any role that can connect erase + // any tenant's record. Revoked, and granted back only to the role applying the migration. + Assert.Equal(2, CountOccurrences(statements, "SECURITY DEFINER")); + Assert.Equal(2, CountOccurrences(statements, "FROM PUBLIC;")); + Assert.Equal(2, CountOccurrences(statements, "TO CURRENT_USER;")); + foreach (var purge in new[] + { + "agent_experience.purge_experience_record(\n uuid, text, text, text, text, text, text, bigint, timestamptz)", + "agent_experience.purge_expired_grants(\n text, text, text, text, text, text, timestamptz, integer)", + }) + { + Assert.Contains($"REVOKE ALL ON FUNCTION {purge} FROM PUBLIC;", statements, StringComparison.Ordinal); + Assert.Contains($"GRANT EXECUTE ON FUNCTION {purge} TO CURRENT_USER;", statements, StringComparison.Ordinal); + } + + // The documentation fixes this story's reviewers asked for, pinned so they cannot quietly go + // back to reassuring: the dead heap tuple still holds the erased text, the index builds are not + // free, and payload_version is on the retained list rather than an exception to it. + Assert.Contains("STILL CARRIES THE ERASED TEXT", script, StringComparison.Ordinal); + Assert.Contains("VACUUM (VERBOSE) agent_experience.experience_records;", script, StringComparison.Ordinal); + Assert.Contains("CREATE INDEX CONCURRENTLY IF NOT EXISTS ix_experience_records_live_by_age", script, StringComparison.Ordinal); + Assert.Contains(" payload_version.", script, StringComparison.Ordinal); + Assert.Contains("ADAPTER-ENFORCED", script, StringComparison.Ordinal); + Assert.Contains("SCHEMA-ENFORCED", script, StringComparison.Ordinal); + + // 0010 is applied last, which the migrator relies on for ordinal name ordering. + Assert.Equal( + PostgresExperienceRecordSchema.DeleteAndExpireScriptName, + PostgresExperienceRecordSchema.ScriptNames[^1]); Assert.Equal( PostgresExperienceRecordSchema.ScriptNames.Order(StringComparer.Ordinal), PostgresExperienceRecordSchema.ScriptNames); } + /// + /// The body of reject_record_removal alone, so "it takes no marker" is asserted about that + /// function rather than about a script that mentions the marker several times elsewhere. + /// + private static string RejectRecordRemovalBody(string statements) + { + const string Start = "CREATE OR REPLACE FUNCTION agent_experience.reject_record_removal()"; + var from = statements.IndexOf(Start, StringComparison.Ordinal); + Assert.True(from >= 0, "0010 no longer defines agent_experience.reject_record_removal()."); + + var to = statements.IndexOf("$body$ LANGUAGE plpgsql;", from, StringComparison.Ordinal); + Assert.True(to > from, "agent_experience.reject_record_removal() has no terminated body."); + return statements[from..to]; + } + private static int CountOccurrences(string text, string value) { var count = 0; diff --git a/tests/AgentExperience.Storage.Postgres.Tests/PlainPostgresMigrationTests.cs b/tests/AgentExperience.Storage.Postgres.Tests/PlainPostgresMigrationTests.cs index 3a04800..726e382 100644 --- a/tests/AgentExperience.Storage.Postgres.Tests/PlainPostgresMigrationTests.cs +++ b/tests/AgentExperience.Storage.Postgres.Tests/PlainPostgresMigrationTests.cs @@ -74,21 +74,91 @@ public async Task The_base_schema_migrates_on_a_PostgreSQL_without_pgvector_avai } [Fact] - public void No_script_this_package_ships_creates_an_extension() + public void No_script_this_package_ships_creates_an_extension_or_depends_on_the_vectors_table() { foreach (var scriptName in PostgresExperienceRecordSchema.ScriptNames) { - var statements = string.Join( - '\n', - PostgresExperienceRecordSchema.GetScript(scriptName) - .Split('\n') - .Where(line => !line.TrimStart().StartsWith("--", StringComparison.Ordinal))); - - Assert.DoesNotContain("CREATE EXTENSION", statements, StringComparison.OrdinalIgnoreCase); - Assert.DoesNotContain("experience_embeddings", statements, StringComparison.Ordinal); + var lines = PostgresExperienceRecordSchema.GetScript(scriptName) + .Split('\n') + .Where(line => !line.TrimStart().StartsWith("--", StringComparison.Ordinal)) + .ToArray(); + + Assert.DoesNotContain("CREATE EXTENSION", string.Join('\n', lines), StringComparison.OrdinalIgnoreCase); + + // The vectors package owns experience_embeddings, and this package must not come to depend + // on it. 0010's erasure still has to remove a record's embedding where one exists, so every + // mention of that table sits behind a to_regclass guard and is issued through EXECUTE: a + // base-only database never parses it, which PlainPostgres proves by migrating without + // pgvector available at all. + foreach (var line in lines.Where(line => line.Contains("experience_embeddings", StringComparison.Ordinal))) + { + Assert.True( + line.Contains("to_regclass", StringComparison.Ordinal) || line.Contains("EXECUTE", StringComparison.Ordinal), + $"{scriptName} names experience_embeddings outside a to_regclass guard: {line.Trim()}"); + } } } + [Fact] + public async Task The_erasure_skips_the_embedding_step_here_and_refuses_to_run_against_a_divergent_one() + { + await ExperienceSchemaMigrator.MigrateAsync(DataSource, CancellationToken.None); + + // A base-only database: no vector extension, no embedding table, and an erasure that simply does + // not perform step 8. This is what the to_regclass guard is *for*. + var skipped = Guid.NewGuid(); + await SeedRecordAsync(skipped); + Assert.Equal("Deleted", await PurgeAsync(skipped)); + + // Now the case the guard did not cover: a relation under that name whose shape is not 0004's. + // Without a column check the EXECUTE fails with a bare undefined_column mid-erasure -- safe, + // because the whole thing is one transaction, but undiagnosable, and the header presents the + // guard as tolerating the vectors package's *absence*, which is not the same as its divergence. + await using (var divergent = DataSource.CreateCommand( + "CREATE TABLE agent_experience.experience_embeddings (record_id uuid NOT NULL PRIMARY KEY)")) + { + await divergent.ExecuteNonQueryAsync(); + } + + var blocked = Guid.NewGuid(); + await SeedRecordAsync(blocked); + + var refused = await Assert.ThrowsAsync(() => PurgeAsync(blocked)); + + Assert.Equal(PostgresErrorCodes.UndefinedColumn, refused.SqlState); + Assert.Contains("experience_id column", refused.MessageText, StringComparison.Ordinal); + + // Nothing was erased: the record still carries its payload, so an operator who reconciles the + // embedding table and retries loses nothing. + Assert.Equal(1L, await ScalarAsync( + $"SELECT count(*) FROM agent_experience.experience_records WHERE experience_id = '{blocked}' " + + "AND deleted_at IS NULL AND payload <> '{}'::jsonb")); + + await using var cleanup = DataSource.CreateCommand("DROP TABLE agent_experience.experience_embeddings"); + await cleanup.ExecuteNonQueryAsync(); + } + + private async Task SeedRecordAsync(Guid experienceId) + { + await using var command = DataSource.CreateCommand( + "INSERT INTO agent_experience.experience_records (experience_id, source_run_id, tenant_id, " + + "application_id, project_id, task_id, status, reuse_confidence, supporting_validations, " + + "contradictions, revision, created_at, updated_at, payload_version, payload) VALUES " + + "(@id, @id, 'tenant-plain', 'app-1', 'project-1', 'task-1', 'Validated', 0, 0, 0, 0, now(), now(), 1, " + + "'{\"taskSummary\":\"still here\"}'::jsonb)"); + command.Parameters.Add(new NpgsqlParameter("id", experienceId)); + Assert.Equal(1, await command.ExecuteNonQueryAsync()); + } + + private async Task PurgeAsync(Guid experienceId) + { + await using var command = DataSource.CreateCommand( + "SELECT purge_outcome FROM agent_experience.purge_experience_record(" + + "@id, 'tenant-plain', 'app-1', 'project-1', NULL, NULL, NULL, NULL, now())"); + command.Parameters.Add(new NpgsqlParameter("id", experienceId)); + return await command.ExecuteScalarAsync(); + } + private async Task ScalarAsync(string sql) { await using var command = DataSource.CreateCommand(sql); diff --git a/tests/AgentExperience.Storage.Postgres.Tests/PostgresDeletionTests.cs b/tests/AgentExperience.Storage.Postgres.Tests/PostgresDeletionTests.cs new file mode 100644 index 0000000..18a5ecd --- /dev/null +++ b/tests/AgentExperience.Storage.Postgres.Tests/PostgresDeletionTests.cs @@ -0,0 +1,1826 @@ +using AgentExperience.Core.Confidence; +using AgentExperience.Core.Feedback; +using AgentExperience.Core.Lifecycle; +using Npgsql; +using NpgsqlTypes; +using static AgentExperience.Storage.Postgres.Tests.TestRecords; + +namespace AgentExperience.Storage.Postgres.Tests; + +/// +/// Story 4.5 against a real PostgreSQL 16 container: the one destructive operation this library has. +/// Every claim here is proved against rows in a database rather than reasoned about -- what erasure +/// removes, what it deliberately keeps, that a foreign scope cannot tell a refusal from an absence, that +/// a tombstone is terminal for every write path, that the append-only guards stay armed in another +/// session while a purge transaction is open, and that a retention sweep erases exactly what a frozen +/// clock says is past the cutoff. Each test uses its own random tenant, so tests sharing the container +/// never see each other's rows. +/// +[Collection(PostgresCollection.Name)] +public sealed class PostgresDeletionTests +{ + private const string Administrator = "sharing-administrator"; + + /// The literal 0010 writes into a tombstone's status column. No ExperienceStatus names it. + private const string TombstoneStatus = "Deleted"; + + /// The literal 0010 writes into a tombstone's task_id, which cannot be blank. + private const string TombstoneTaskId = "(deleted)"; + + /// + /// How long a concurrency test lets a second writer reach the row lock before the purge holding it + /// commits. It is not a correctness bound -- a writer that arrives late simply reads the committed + /// tombstone and loses for the other reason -- only a way of making the interesting interleaving the + /// usual one rather than the rare one. + /// + private static readonly TimeSpan OverlapWindow = TimeSpan.FromMilliseconds(300); + + private readonly PostgresFixture _fixture; + private readonly PostgresExperienceRecordStore _store; + private readonly PostgresExperienceGrantStore _grants; + private readonly PostgresExperienceReuseFeedbackStore _ledger; + private readonly PostgresExperienceGrantAccessLog _access; + private readonly ExperienceLifecycleService _lifecycle; + private readonly ExperienceReuseFeedbackService _feedback; + + public PostgresDeletionTests(PostgresFixture fixture) + { + _fixture = fixture; + _access = new PostgresExperienceGrantAccessLog(fixture.DataSource); + _store = new PostgresExperienceRecordStore( + fixture.DataSource, + onGrantsUnavailable: null, + auditing: new ExperienceGrantAuditing(_access, _ => { })); + _grants = new PostgresExperienceGrantStore(fixture.DataSource); + _ledger = new PostgresExperienceReuseFeedbackStore(fixture.DataSource); + _lifecycle = new ExperienceLifecycleService(_store); + _feedback = new ExperienceReuseFeedbackService(_ledger, _lifecycle); + } + + // ------------------------------------------------------------------ the erasure itself + + [Fact] + public async Task Deleting_a_record_erases_every_row_that_named_it_and_leaves_exactly_the_tombstone() + { + var tenant = NewTenant(); + var auth = Authorize(tenant); + var owner = Scope(tenant, team: "team-a"); + var recipient = Scope(tenant, team: "team-b"); + var record = await PopulatedAsync(auth, owner, recipient); + + // Everything the record accumulated is really there before the delete, or the assertions below + // would pass over an empty database. + Assert.True(await CountAsync("lifecycle_events", record.ExperienceId) > 0); + Assert.True(await CountAsync("confidence_evidence", record.ExperienceId) > 0); + Assert.True(await CountAsync("reuse_feedback_exposures", record.ExperienceId) > 0); + Assert.True(await CountAsync("experience_grants", record.ExperienceId) > 0); + Assert.True(await CountGrantEventsAsync(record.ExperienceId) > 0); + Assert.Equal(1, await CountAsync("experience_grant_access", record.ExperienceId)); + var feedbackId = Assert.Single(await FeedbackIdsAsync(record.ExperienceId)); + + var deleted = await _store.DeleteAsync(auth, owner, record.ExperienceId, CancellationToken.None); + + Assert.Equal(ExperienceStoreOutcome.Deleted, deleted.Outcome); + Assert.Empty(deleted.Errors); + + // One erasure, one frozen order, and nothing that named the record survives it. + Assert.Equal(0, await CountAsync("confidence_evidence", record.ExperienceId)); + Assert.Equal(0, await CountAsync("reuse_feedback_exposures", record.ExperienceId)); + Assert.Equal(0, await CountFeedbackAsync(feedbackId)); + Assert.Equal(0, await CountGrantEventsAsync(record.ExperienceId)); + Assert.Equal(0, await CountAsync("experience_grants", record.ExperienceId)); + Assert.Equal(0, await CountAsync("lifecycle_events", record.ExperienceId)); + + // ...except the access trail, which is retained on purpose: it names a grant and a principal, + // carries no payload, and is the answer to "who read this before it was deleted". It has to stay + // *readable* to be that answer, so it is read back through the port rather than counted in SQL: + // the ledger joins no record, so erasing one cannot make its trail unreadable. + Assert.Equal(1, await CountAsync("experience_grant_access", record.ExperienceId)); + + var trail = await _access.QueryAsync( + auth, new ExperienceGrantAccessQuery(owner, record.ExperienceId), CancellationToken.None); + Assert.Equal(ExperienceStoreOutcome.Found, trail.Outcome); + Assert.Equal(record.ExperienceId, Assert.Single(trail.Accesses).ExperienceId); + + // The tombstone carries the retained list and nothing else. Every other column is a fixed, + // content-free value -- including the timestamps, so it cannot say when the work happened. + var tombstone = await ReadTombstoneAsync(record.ExperienceId); + + Assert.Equal(owner.TenantId, tombstone.TenantId); + Assert.Equal(owner.ApplicationId, tombstone.ApplicationId); + Assert.Equal(owner.ProjectId, tombstone.ProjectId); + Assert.Equal(owner.TeamId, tombstone.TeamId); + Assert.Null(tombstone.AgentId); + Assert.Null(tombstone.UserId); + Assert.Equal(deleted.Revision, tombstone.Revision); + Assert.Equal(TombstoneStatus, tombstone.Status); + Assert.Equal(TombstoneTaskId, tombstone.TaskId); + Assert.NotNull(tombstone.DeletedAt); + + Assert.Equal(Guid.Empty, tombstone.SourceRunId); + Assert.Equal("{}", tombstone.Payload); + Assert.Equal(0d, tombstone.ReuseConfidence); + Assert.Equal(0, tombstone.SupportingValidations); + Assert.Equal(0, tombstone.Contradictions); + Assert.Equal(tombstone.DeletedAt, tombstone.CreatedAt); + Assert.Equal(tombstone.DeletedAt, tombstone.UpdatedAt); + + // The generated search vector regenerates from the placeholder alone, so the record's text is + // gone from the index as well as from the column -- no separate index maintenance, by design. + Assert.Equal("'delet':1", tombstone.SearchVector); + + // And the revision moved exactly once, the way every other change to a record does. + Assert.Equal(record.Revision + 1, tombstone.Revision); + } + + [Fact] + public async Task A_submission_that_named_other_records_survives_with_only_this_exposure_gone() + { + var tenant = NewTenant(); + var auth = Authorize(tenant); + var scope = Scope(tenant); + var erased = await ValidatedAsync(auth, scope); + var kept = await ValidatedAsync(auth, scope); + + var feedback = Feedback(scope, [erased.ExperienceId, kept.ExperienceId]); + Assert.Equal( + ExperienceReuseFeedbackOutcome.Recorded, + (await _feedback.RecordAsync(auth, feedback, CancellationToken.None)).Outcome); + + Assert.Equal( + ExperienceStoreOutcome.Deleted, + (await _store.DeleteAsync(auth, scope, erased.ExperienceId, CancellationToken.None)).Outcome); + + // The submission still describes the other record it named, so its row stays; only the exposure + // of the erased record is removed. A submission left with no exposures at all is deleted -- that + // is the case the first test covers. + Assert.Equal(1, await CountFeedbackAsync(feedback.FeedbackId)); + Assert.Equal(0, await CountAsync("reuse_feedback_exposures", erased.ExperienceId)); + Assert.Equal(1, await CountAsync("reuse_feedback_exposures", kept.ExperienceId)); + } + + [Fact] + public async Task The_projection_guard_accepts_the_tombstone_from_every_prior_status() + { + // The open risk the spec flagged: the tombstone's final UPDATE has to satisfy the very guard + // that protects the projection, from wherever the record happened to be. It advances the + // revision like any other transition, so every prior status is accepted -- proved, not assumed. + var tenant = NewTenant(); + var auth = Authorize(tenant); + var scope = Scope(tenant); + + foreach (var status in Enum.GetValues()) + { + var record = Minimal(scope); + Assert.Equal(ExperienceStoreOutcome.Created, (await _store.CreateAsync(auth, record, CancellationToken.None)).Outcome); + + // Straight to the status under test, through the store's own guarded commit. A superseding + // event has to name a replacement, so that one gets an eligible record to point at. + Guid? replacement = null; + if (status == ExperienceStatus.Superseded) + { + replacement = (await ValidatedAsync(auth, scope)).ExperienceId; + } + + var commit = await _store.CommitLifecycleEventAsync( + auth, + scope, + Event(record.ExperienceId, ExperienceStatus.Candidate, status, 0, replacement: replacement), + CancellationToken.None); + + Assert.Equal(ExperienceStoreOutcome.Committed, commit.Outcome); + + var deleted = await _store.DeleteAsync(auth, scope, record.ExperienceId, CancellationToken.None); + + Assert.Equal(ExperienceStoreOutcome.Deleted, deleted.Outcome); + Assert.Equal(TombstoneStatus, (await ReadTombstoneAsync(record.ExperienceId)).Status); + } + } + + // ------------------------------------------------------------------ refusals + + [Fact] + public async Task A_delete_naming_another_scope_is_the_same_answer_as_one_naming_nothing() + { + var tenant = NewTenant(); + var auth = Authorize(tenant); + var mine = Scope(tenant, team: "team-a"); + var theirs = Scope(tenant, team: "team-b"); + var record = await ValidatedAsync(auth, theirs); + + var foreign = await _store.DeleteAsync(auth, mine, record.ExperienceId, CancellationToken.None); + var absent = await _store.DeleteAsync(auth, mine, Guid.NewGuid(), CancellationToken.None); + + // Identical outcome, identical revision, identical (empty) errors: nothing about the record's + // existence reaches a scope that does not own it. + Assert.Equal(ExperienceStoreOutcome.NotFound, foreign.Outcome); + Assert.Equal(absent.Outcome, foreign.Outcome); + Assert.Equal(absent.Revision, foreign.Revision); + Assert.Equal(absent.Errors, foreign.Errors); + + // And the record it named is untouched. + var stored = await _store.GetAsync(auth, theirs, record.ExperienceId, CancellationToken.None); + Assert.Equal(ExperienceStoreOutcome.Found, stored.Outcome); + Assert.Null((await ReadTombstoneAsync(record.ExperienceId)).DeletedAt); + + // A scope outside the host-established authorization never reaches storage at all. + var denied = await _store.DeleteAsync(Authorize(NewTenant()), theirs, record.ExperienceId, CancellationToken.None); + Assert.Equal(ExperienceStoreOutcome.Denied, denied.Outcome); + Assert.Equal(ExperienceStoreOutcome.Found, (await _store.GetAsync(auth, theirs, record.ExperienceId, CancellationToken.None)).Outcome); + } + + [Fact] + public async Task Deleting_twice_is_a_success_that_touches_nothing() + { + var tenant = NewTenant(); + var auth = Authorize(tenant); + var scope = Scope(tenant); + var record = await ValidatedAsync(auth, scope); + + var first = await _store.DeleteAsync(auth, scope, record.ExperienceId, CancellationToken.None); + var tombstone = await ReadTombstoneAsync(record.ExperienceId); + + var again = await _store.DeleteAsync(auth, scope, record.ExperienceId, CancellationToken.None); + + Assert.Equal(ExperienceStoreOutcome.Deleted, again.Outcome); + Assert.Equal(first.Revision, again.Revision); + + // Not one column moved -- not the revision, not deleted_at, not updated_at. A second delete that + // re-stamped the tombstone would make "when was this erased" a value any caller could refresh. + var after = await ReadTombstoneAsync(record.ExperienceId); + Assert.Equal(tombstone, after); + } + + [Fact] + public async Task A_stale_expected_revision_refuses_the_delete_and_erases_nothing() + { + var tenant = NewTenant(); + var auth = Authorize(tenant); + var scope = Scope(tenant); + var record = await ValidatedAsync(auth, scope); + + var stale = await _store.DeleteAsync(auth, scope, record.ExperienceId, expectedRevision: 0, CancellationToken.None); + + Assert.Equal(ExperienceStoreOutcome.StaleRevision, stale.Outcome); + Assert.Equal(1, stale.Revision); + Assert.Equal(1, await CountAsync("lifecycle_events", record.ExperienceId)); + Assert.Null((await ReadTombstoneAsync(record.ExperienceId)).DeletedAt); + + // ...and the same call against the revision the record is really at erases it. + var deleted = await _store.DeleteAsync(auth, scope, record.ExperienceId, expectedRevision: 1, CancellationToken.None); + Assert.Equal(ExperienceStoreOutcome.Deleted, deleted.Outcome); + Assert.Equal(2, deleted.Revision); + + // A stale revision in another scope is still NotFound: the guard never reveals the record. + var other = await ValidatedAsync(auth, Scope(tenant, team: "team-c")); + var foreign = await _store.DeleteAsync(auth, scope, other.ExperienceId, expectedRevision: 0, CancellationToken.None); + Assert.Equal(ExperienceStoreOutcome.NotFound, foreign.Outcome); + } + + [Fact] + public async Task A_malformed_delete_or_sweep_is_refused_before_any_connection_opens() + { + var tenant = NewTenant(); + var auth = Authorize(tenant); + var scope = Scope(tenant); + + var offline = new PostgresExperienceRecordStore(Unreachable()); + + var empty = await offline.DeleteAsync(auth, scope, Guid.Empty, CancellationToken.None); + Assert.Equal(ExperienceStoreOutcome.Invalid, empty.Outcome); + Assert.Equal("ExperienceId", Assert.Single(empty.Errors).Path); + + var negative = await offline.DeleteAsync(auth, scope, Guid.NewGuid(), expectedRevision: -1, CancellationToken.None); + Assert.Equal("ExpectedRevision", Assert.Single(negative.Errors).Path); + + // There is no retention age that means "delete everything", and no unbounded batch. + var forever = await offline.SweepExpiredAsync(auth, scope, TimeSpan.Zero, 10, CancellationToken.None); + Assert.Equal(ExperienceStoreOutcome.Invalid, forever.Outcome); + Assert.Equal("RetentionAge", Assert.Single(forever.Errors).Path); + + var unbounded = await offline.SweepExpiredAsync( + auth, scope, TimeSpan.FromDays(1), PostgresExperienceRecordStore.MaxSweepBatchSize + 1, CancellationToken.None); + Assert.Equal("BatchSize", Assert.Single(unbounded.Errors).Path); + + var denied = await offline.SweepExpiredAsync(Authorize(NewTenant()), scope, TimeSpan.FromDays(1), 10, CancellationToken.None); + Assert.Equal(ExperienceStoreOutcome.Denied, denied.Outcome); + } + + // ------------------------------------------------------------------ a tombstone is terminal + + [Fact] + public async Task Every_write_path_refuses_a_tombstone_rather_than_resurrecting_it() + { + var tenant = NewTenant(); + var auth = Authorize(tenant); + var owner = Scope(tenant, team: "team-a"); + var recipient = Scope(tenant, team: "team-b"); + var record = await ValidatedAsync(auth, owner); + + Assert.Equal( + ExperienceStoreOutcome.Deleted, + (await _store.DeleteAsync(auth, owner, record.ExperienceId, CancellationToken.None)).Outcome); + + // A create under the erased ID collides with the tombstone, exactly as it would with any stored + // record, and says nothing about which scope holds it. + var recreated = await _store.CreateAsync(auth, Minimal(owner, record.ExperienceId), CancellationToken.None); + Assert.Equal(ExperienceStoreOutcome.Conflict, recreated.Outcome); + + // A late lifecycle commit is Deleted, not StaleRevision and not NotFound: within its own scope a + // host can tell "erased" from "never existed", and neither answer moves the tombstone. + var commit = await _store.CommitLifecycleEventAsync( + auth, owner, Event(record.ExperienceId, ExperienceStatus.Validated, ExperienceStatus.Revoked, 1), CancellationToken.None); + Assert.Equal(ExperienceStoreOutcome.Deleted, commit.Outcome); + Assert.Equal(0, await CountAsync("lifecycle_events", record.ExperienceId)); + + // A late confidence submission is refused for the same reason and writes no ledger row either. + var withEvidence = await _store.CommitLifecycleEventAsync( + auth, + owner, + Event(record.ExperienceId, ExperienceStatus.Validated, ExperienceStatus.Reinforced, 1) with + { + Confidence = new ConfidenceUpdate( + EvidenceId: Guid.NewGuid(), + Kind: ConfidenceEvidenceKind.Supporting, + Source: ConfidenceEvidenceSource.Machine, + RunId: Guid.NewGuid(), + VerificationRoundId: Guid.NewGuid(), + ReviewerIdentity: null, + RuleVersion: ReuseConfidenceHeuristic.RuleVersion, + PriorReuseConfidence: 0.5, + NewReuseConfidence: 0.6, + PriorSupportingValidations: 1, + NewSupportingValidations: 2, + PriorContradictions: 0, + NewContradictions: 0), + }, + CancellationToken.None); + Assert.Equal(ExperienceStoreOutcome.Deleted, withEvidence.Outcome); + Assert.Equal(0, await CountAsync("confidence_evidence", record.ExperienceId)); + + // Feedback naming the tombstone is refused, by position, with nothing written. + var feedback = Feedback(owner, [record.ExperienceId]); + var recorded = await _ledger.RecordAsync(auth, Submission(feedback), CancellationToken.None); + Assert.Equal(ExperienceReuseFeedbackStoreOutcome.Invalid, recorded.Outcome); + Assert.Equal("Exposures[0].ExperienceId", Assert.Single(recorded.Errors).Path); + Assert.Equal(0, await CountFeedbackAsync(feedback.FeedbackId)); + Assert.Equal(0, await CountAsync("reuse_feedback_exposures", record.ExperienceId)); + + // A grant over the tombstone is NotFound: there is nothing left to share. + var granted = await _grants.CreateAsync( + auth, + new GrantAdministration(Administrator, DateTimeOffset.UtcNow), + new ExperienceGrantRequest( + Guid.NewGuid(), record.ExperienceId, owner, recipient, "after the erasure", DateTimeOffset.UtcNow.AddHours(1)), + CancellationToken.None); + Assert.Equal(ExperienceGrantOutcome.NotFound, granted.Outcome); + Assert.Equal(0, await CountAsync("experience_grants", record.ExperienceId)); + } + + [Fact] + public async Task Every_read_path_reports_the_tombstone_as_erased_or_not_at_all() + { + var tenant = NewTenant(); + var auth = Authorize(tenant); + var scope = Scope(tenant); + var record = await ValidatedAsync(auth, scope); + var survivor = await ValidatedAsync(auth, scope); + + Assert.Equal( + ExperienceStoreOutcome.Deleted, + (await _store.DeleteAsync(auth, scope, record.ExperienceId, CancellationToken.None)).Outcome); + + // Named reads inside the owning scope say "erased" and hand back nothing. + var get = await _store.GetAsync(auth, scope, record.ExperienceId, CancellationToken.None); + Assert.Equal(ExperienceStoreOutcome.Deleted, get.Outcome); + Assert.Null(get.Record); + + var history = await _store.GetFirstHistoryPageAsync(auth, scope, record.ExperienceId, CancellationToken.None); + Assert.Equal(ExperienceStoreOutcome.Deleted, history.Outcome); + Assert.Empty(history.Events); + + // Enumerations simply do not contain it. A tombstone has no payload to return. + var listed = await _store.QueryAsync(auth, new ExperienceRecordQuery(scope), CancellationToken.None); + Assert.Equal([survivor.ExperienceId], listed.Records.Select(r => r.ExperienceId)); + + var search = await new PostgresExperienceCandidateSource(_fixture.DataSource).SearchAsync( + auth, + new ExperienceCandidateQuery(scope, "task-1 deleted", [ExperienceStatus.Validated, ExperienceStatus.Reinforced], 0, 50), + CancellationToken.None); + Assert.DoesNotContain(record.ExperienceId, search.Candidates.Select(c => c.Record.ExperienceId)); + + // And it can neither be superseded nor named as a replacement. + var asRecord = await _store.CheckSupersessionAsync(auth, scope, record.ExperienceId, survivor.ExperienceId, CancellationToken.None); + Assert.Equal(ExperienceSupersessionOutcome.RecordNotFound, asRecord.Outcome); + + var asReplacement = await _store.CheckSupersessionAsync(auth, scope, survivor.ExperienceId, record.ExperienceId, CancellationToken.None); + Assert.Equal(ExperienceSupersessionOutcome.ReplacementNotFound, asReplacement.Outcome); + + // A foreign scope is told nothing at all -- not even that the ID was once used here. + var foreign = await _store.GetAsync(auth, Scope(tenant, team: "team-z"), record.ExperienceId, CancellationToken.None); + Assert.Equal(ExperienceStoreOutcome.NotFound, foreign.Outcome); + } + + [Fact] + public async Task The_database_refuses_a_tombstone_written_or_moved_outside_the_purge_path() + { + var tenant = NewTenant(); + var auth = Authorize(tenant); + var scope = Scope(tenant); + var live = await ValidatedAsync(auth, scope); + var erased = await ValidatedAsync(auth, scope); + + Assert.Equal( + ExperienceStoreOutcome.Deleted, + (await _store.DeleteAsync(auth, scope, erased.ExperienceId, CancellationToken.None)).Outcome); + + // Erasure has one code path. A direct statement cannot mark a record erased... + var marked = await Assert.ThrowsAsync(() => ExecuteAsync( + "UPDATE agent_experience.experience_records SET deleted_at = now(), revision = revision + 1 " + + "WHERE experience_id = @id", + live.ExperienceId)); + + // ...and cannot change a tombstone once it exists, in any way, marker or no marker. + var moved = await Assert.ThrowsAsync(() => ExecuteAsync( + "UPDATE agent_experience.experience_records SET updated_at = now() WHERE experience_id = @id", + erased.ExperienceId)); + + var revived = await Assert.ThrowsAsync(() => ExecuteAsync( + "UPDATE agent_experience.experience_records SET deleted_at = NULL, revision = revision + 1 " + + "WHERE experience_id = @id", + erased.ExperienceId)); + + Assert.All([marked, moved, revived], ex => Assert.Equal(PostgresErrorCodes.InsufficientPrivilege, ex.SqlState)); + + // The tombstone-shape CHECK is the other half: "erased" is one shape, never a flag set over a + // payload that is still there. + var halfErased = await Assert.ThrowsAsync(() => ExecuteAsync( + "INSERT INTO agent_experience.experience_records (experience_id, source_run_id, tenant_id, application_id, " + + "project_id, task_id, status, reuse_confidence, supporting_validations, contradictions, revision, " + + "created_at, updated_at, payload_version, payload, deleted_at) VALUES (@id, @id, @tenant, 'app-1', " + + "'project-1', 'task-1', 'Validated', 0, 0, 0, 0, now(), now(), 1, '{\"taskSummary\":\"still here\"}'::jsonb, now())", + Guid.NewGuid(), + tenant)); + + Assert.Equal(PostgresErrorCodes.CheckViolation, halfErased.SqlState); + Assert.Equal("experience_records_tombstone_shape", halfErased.ConstraintName); + } + + [Fact] + public async Task The_append_only_guards_stay_armed_in_another_session_while_a_purge_is_open() + { + var tenant = NewTenant(); + var auth = Authorize(tenant); + var scope = Scope(tenant); + var erasing = await ValidatedAsync(auth, scope); + var bystander = await ValidatedAsync(auth, scope); + await ApplyEvidenceAsync(auth, scope, bystander.ExperienceId); + + // One connection holds an open purge transaction -- the marker is set, the rows are gone, and + // nothing is committed yet. + await using var purging = await _fixture.DataSource.OpenConnectionAsync(); + await using var transaction = await purging.BeginTransactionAsync(); + + await using (var purge = new NpgsqlCommand( + "SELECT purge_outcome FROM agent_experience.purge_experience_record(" + + "@id, @tenant, 'app-1', 'project-1', NULL, NULL, NULL, NULL, now())", + purging, + transaction)) + { + purge.Parameters.Add(new NpgsqlParameter("id", erasing.ExperienceId)); + purge.Parameters.Add(new NpgsqlParameter("tenant", NpgsqlDbType.Text) { TypedValue = tenant }); + Assert.Equal("Deleted", await purge.ExecuteScalarAsync()); + } + + // Meanwhile, on a different connection, the guards are exactly as armed as they always are. This + // is the whole point of a transaction-scoped marker over DISABLE TRIGGER: the window a purge + // opens is invisible to every other session. + var deleteLog = await Assert.ThrowsAsync(() => ExecuteAsync( + "DELETE FROM agent_experience.lifecycle_events WHERE experience_id = @id", bystander.ExperienceId)); + var rewriteLog = await Assert.ThrowsAsync(() => ExecuteAsync( + "UPDATE agent_experience.lifecycle_events SET producer = 'nobody' WHERE experience_id = @id", bystander.ExperienceId)); + var evidence = await Assert.ThrowsAsync(() => ExecuteAsync( + "DELETE FROM agent_experience.confidence_evidence WHERE experience_id = @id", bystander.ExperienceId)); + + Assert.All( + [deleteLog, rewriteLog, evidence], + ex => Assert.Equal(PostgresErrorCodes.InsufficientPrivilege, ex.SqlState)); + + // Rolling the purge back leaves the record it was erasing whole: the erasure is one transaction, + // so a failure part-way through erases nothing at all. + await transaction.RollbackAsync(); + + Assert.Equal(ExperienceStoreOutcome.Found, (await _store.GetAsync(auth, scope, erasing.ExperienceId, CancellationToken.None)).Outcome); + Assert.Equal(1, await CountAsync("lifecycle_events", erasing.ExperienceId)); + + // TRUNCATE is asserted after the rollback rather than during the purge, and for a reason worth + // stating: it takes an ACCESS EXCLUSIVE lock, so it would queue behind any open writer whatever + // the guards said, and a test that "passed" by timing out would prove nothing. + var truncate = await Assert.ThrowsAsync(() => ExecuteAsync( + "TRUNCATE agent_experience.lifecycle_events", id: null)); + Assert.Equal(PostgresErrorCodes.InsufficientPrivilege, truncate.SqlState); + } + + [Fact] + public async Task The_marker_does_not_outlive_the_purge_function_inside_its_own_transaction() + { + var tenant = NewTenant(); + var auth = Authorize(tenant); + var scope = Scope(tenant); + var erasing = await ValidatedAsync(auth, scope); + var bystander = await ValidatedAsync(auth, scope); + + await using var connection = await _fixture.DataSource.OpenConnectionAsync(); + await using var transaction = await connection.BeginTransactionAsync(); + + await using (var purge = new NpgsqlCommand( + "SELECT purge_outcome FROM agent_experience.purge_experience_record(" + + "@id, @tenant, 'app-1', 'project-1', NULL, NULL, NULL, NULL, now())", + connection, + transaction)) + { + purge.Parameters.Add(new NpgsqlParameter("id", erasing.ExperienceId)); + purge.Parameters.Add(new NpgsqlParameter("tenant", NpgsqlDbType.Text) { TypedValue = tenant }); + await purge.ExecuteScalarAsync(); + } + + // The function declares a SET for the same variable, so the marker is restored when it returns + // rather than at the end of the transaction: even the purging connection cannot go on to delete + // another record's log with it. + await using var afterwards = new NpgsqlCommand( + "DELETE FROM agent_experience.lifecycle_events WHERE experience_id = @id", connection, transaction); + afterwards.Parameters.Add(new NpgsqlParameter("id", bystander.ExperienceId)); + + var refused = await Assert.ThrowsAsync(() => afterwards.ExecuteNonQueryAsync()); + Assert.Equal(PostgresErrorCodes.InsufficientPrivilege, refused.SqlState); + + await transaction.RollbackAsync(); + } + + [Fact] + public async Task The_erasure_is_out_of_reach_of_a_role_that_was_never_granted_it() + { + // The most severe thing this story could have shipped: both purge functions are SECURITY + // DEFINER, and PostgreSQL grants EXECUTE on a new function to PUBLIC by default. Left at that + // default, a SELECT-only reporting role -- no DELETE, no UPDATE, anywhere -- could read an + // experience_id and a scope out of the record table and permanently erase that record, in any + // tenant. This proves the revoke, from a connection that is not the owner, which is the only + // connection that can prove it. + var tenant = NewTenant(); + var auth = Authorize(tenant); + var scope = Scope(tenant); + var record = await ValidatedAsync(auth, scope); + + await using var reporterSource = await _fixture.CreateRoleAsync( + "reporter", + "GRANT USAGE ON SCHEMA agent_experience TO {role}", + "GRANT SELECT ON ALL TABLES IN SCHEMA agent_experience TO {role}"); + + await using var reporter = await reporterSource.OpenConnectionAsync(); + + // It really can read the ID and the scope it would need. That is the whole premise. + await using (var read = new NpgsqlCommand( + "SELECT tenant_id FROM agent_experience.experience_records WHERE experience_id = @id", reporter)) + { + read.Parameters.Add(new NpgsqlParameter("id", record.ExperienceId)); + Assert.Equal(tenant, await read.ExecuteScalarAsync()); + } + + await using (var purge = new NpgsqlCommand( + "SELECT purge_outcome FROM agent_experience.purge_experience_record(" + + "@id, @tenant, 'app-1', 'project-1', NULL, NULL, NULL, NULL, now())", + reporter)) + { + purge.Parameters.Add(new NpgsqlParameter("id", record.ExperienceId)); + purge.Parameters.Add(new NpgsqlParameter("tenant", NpgsqlDbType.Text) { TypedValue = tenant }); + + var denied = await Assert.ThrowsAsync(() => purge.ExecuteScalarAsync()); + Assert.Equal(PostgresErrorCodes.InsufficientPrivilege, denied.SqlState); + } + + // ...and the grant purge is no easier, which matters because it is the unbounded-by-default one. + await using (var grants = new NpgsqlCommand( + "SELECT agent_experience.purge_expired_grants(@tenant, 'app-1', 'project-1', NULL, NULL, NULL, now(), NULL)", + reporter)) + { + grants.Parameters.Add(new NpgsqlParameter("tenant", NpgsqlDbType.Text) { TypedValue = tenant }); + + var denied = await Assert.ThrowsAsync(() => grants.ExecuteScalarAsync()); + Assert.Equal(PostgresErrorCodes.InsufficientPrivilege, denied.SqlState); + } + + // Nothing was erased, and the owner can still erase. + Assert.Null((await ReadTombstoneAsync(record.ExperienceId)).DeletedAt); + Assert.Equal( + ExperienceStoreOutcome.Deleted, + (await _store.DeleteAsync(auth, scope, record.ExperienceId, CancellationToken.None)).Outcome); + } + + [Fact] + public async Task The_record_row_itself_cannot_be_deleted_or_truncated_by_anybody() + { + // The statement that undoes the whole erasure: deleting the record row orphans an audit trail + // that has no foreign key back to it, and -- worse -- FREES THE ID, so a record recreated under + // it inherits every grant issued over the old content. That is exactly what the tombstone + // exists to prevent, so the schema refuses it rather than the documentation promising it. + var tenant = NewTenant(); + var auth = Authorize(tenant); + var scope = Scope(tenant); + var record = await ValidatedAsync(auth, scope); + + var bare = await Assert.ThrowsAsync(() => ExecuteAsync( + "DELETE FROM agent_experience.experience_records WHERE experience_id = @id", record.ExperienceId)); + Assert.Equal(PostgresErrorCodes.InsufficientPrivilege, bare.SqlState); + + // The marker is not a way round it either: this guard has no exception at all, because the + // erasure never deletes that row. + var marked = await Assert.ThrowsAsync(() => MarkedAsync( + "DELETE FROM agent_experience.experience_records WHERE experience_id = @id", record.ExperienceId)); + Assert.Equal(PostgresErrorCodes.InsufficientPrivilege, marked.SqlState); + + var truncate = await Assert.ThrowsAsync(() => ExecuteAsync( + "TRUNCATE agent_experience.experience_records CASCADE", id: null)); + Assert.Equal(PostgresErrorCodes.InsufficientPrivilege, truncate.SqlState); + + // The record, its history, and its scope are all still there. + Assert.Equal(1, await CountAsync("lifecycle_events", record.ExperienceId)); + Assert.Equal(ExperienceStoreOutcome.Found, (await _store.GetAsync(auth, scope, record.ExperienceId, CancellationToken.None)).Outcome); + + // And after a real erasure the ID is still taken, which is the property the guard protects: a + // create under it collides with the tombstone rather than inheriting the old grants. + Assert.Equal( + ExperienceStoreOutcome.Deleted, + (await _store.DeleteAsync(auth, scope, record.ExperienceId, CancellationToken.None)).Outcome); + Assert.Equal( + ExperienceStoreOutcome.Conflict, + (await _store.CreateAsync(auth, Minimal(scope, record.ExperienceId), CancellationToken.None)).Outcome); + + var tombstoneGone = await Assert.ThrowsAsync(() => ExecuteAsync( + "DELETE FROM agent_experience.experience_records WHERE experience_id = @id", record.ExperienceId)); + Assert.Equal(PostgresErrorCodes.InsufficientPrivilege, tombstoneGone.SqlState); + } + + [Fact] + public async Task A_marked_update_cannot_tombstone_a_record_into_another_scope_or_restamp_it() + { + // The marked exception is shape-checked, and the shape includes the scope. Without that, one + // marked UPDATE could tombstone a record into another tenant: the scope that owned it would then + // see NotFound for its own erased record and a scope that never held it would see Deleted. + var tenant = NewTenant(); + var auth = Authorize(tenant); + var mine = Scope(tenant, team: "team-a"); + var theirs = Scope(tenant, team: "team-b"); + var record = await ValidatedAsync(auth, mine); + + const string Tombstone = + "UPDATE agent_experience.experience_records SET payload = '{}'::jsonb, task_id = '(deleted)', " + + "status = 'Deleted', source_run_id = '00000000-0000-0000-0000-000000000000'::uuid, " + + "reuse_confidence = 0, supporting_validations = 0, contradictions = 0, " + + "created_at = now(), updated_at = now(), deleted_at = now(), revision = revision + 1"; + + var moved = await Assert.ThrowsAsync(() => MarkedAsync( + $"{Tombstone}, team_id = 'team-b' WHERE experience_id = @id", record.ExperienceId)); + + // ...nor keep a timestamp that says when the work happened... + var restamped = await Assert.ThrowsAsync(() => MarkedAsync( + "UPDATE agent_experience.experience_records SET payload = '{}'::jsonb, task_id = '(deleted)', " + + "status = 'Deleted', source_run_id = '00000000-0000-0000-0000-000000000000'::uuid, " + + "reuse_confidence = 0, supporting_validations = 0, contradictions = 0, " + + "updated_at = now(), deleted_at = now(), revision = revision + 1 WHERE experience_id = @id", + record.ExperienceId)); + + // ...nor rewrite the envelope version on the way past... + var reversioned = await Assert.ThrowsAsync(() => MarkedAsync( + $"{Tombstone}, payload_version = payload_version + 1 WHERE experience_id = @id", record.ExperienceId)); + + // ...nor skip a revision, which would leave a gap the event log could never explain. + var jumped = await Assert.ThrowsAsync(() => MarkedAsync( + $"{Tombstone.Replace("revision = revision + 1", "revision = revision + 7", StringComparison.Ordinal)} " + + "WHERE experience_id = @id", + record.ExperienceId)); + + Assert.All( + [moved, restamped, reversioned, jumped], + ex => Assert.Equal(PostgresErrorCodes.InsufficientPrivilege, ex.SqlState)); + + // Nothing moved, and the record is still the owning scope's live record. + Assert.Equal(ExperienceStoreOutcome.Found, (await _store.GetAsync(auth, mine, record.ExperienceId, CancellationToken.None)).Outcome); + Assert.Equal(ExperienceStoreOutcome.NotFound, (await _store.GetAsync(auth, theirs, record.ExperienceId, CancellationToken.None)).Outcome); + + var stored = await ReadTombstoneAsync(record.ExperienceId); + Assert.Null(stored.DeletedAt); + Assert.Equal(mine.TeamId, stored.TeamId); + } + + // ------------------------------------------------------------------ concurrency + + [Fact] + public async Task Two_concurrent_deletes_of_one_record_erase_it_exactly_once() + { + var tenant = NewTenant(); + var auth = Authorize(tenant); + var scope = Scope(tenant); + var record = await PopulatedAsync(auth, scope, Scope(tenant, team: "team-b")); + + var results = await Task.WhenAll( + _store.DeleteAsync(auth, scope, record.ExperienceId, CancellationToken.None), + _store.DeleteAsync(auth, scope, record.ExperienceId, CancellationToken.None)); + + // Both succeed -- erasing an erased record is a success that touches nothing -- and both report + // the same revision, because only one of them moved it. + Assert.All(results, result => Assert.Equal(ExperienceStoreOutcome.Deleted, result.Outcome)); + Assert.Equal(results[0].Revision, results[1].Revision); + + var tombstone = await ReadTombstoneAsync(record.ExperienceId); + Assert.Equal(record.Revision + 1, tombstone.Revision); + Assert.Equal(results[0].Revision, tombstone.Revision); + } + + [Fact] + public async Task A_delete_racing_a_lifecycle_commit_leaves_a_tombstone_and_no_history() + { + var tenant = NewTenant(); + var auth = Authorize(tenant); + var scope = Scope(tenant); + var record = await ValidatedAsync(auth, scope); + + var delete = _store.DeleteAsync(auth, scope, record.ExperienceId, CancellationToken.None); + var commit = _store.CommitLifecycleEventAsync( + auth, scope, Event(record.ExperienceId, ExperienceStatus.Validated, ExperienceStatus.Reinforced, 1), CancellationToken.None); + + var deleted = await delete; + var committed = await commit; + + // Whichever order the two land in, the record ends up erased with nothing left of its history: + // if the commit won, the erasure swept the event it had just written; if the erasure won, the + // commit is refused as Deleted and its event is rolled back with it. + Assert.Equal(ExperienceStoreOutcome.Deleted, deleted.Outcome); + Assert.Contains( + committed.Outcome, + new[] { ExperienceStoreOutcome.Committed, ExperienceStoreOutcome.Deleted, ExperienceStoreOutcome.StaleRevision }); + + Assert.Equal(0, await CountAsync("lifecycle_events", record.ExperienceId)); + Assert.NotNull((await ReadTombstoneAsync(record.ExperienceId)).DeletedAt); + } + + [Fact] + public async Task A_grant_issued_while_a_record_is_being_erased_loses_instead_of_surviving_the_purge() + { + // AD-1, the grant path. experience_grants has no foreign key to experience_records, so nothing + // parks this writer against the erasure by itself: before the row lock it decided against a + // snapshot taken before the purge committed, and left a live 90-day permission over a spent ID. + var tenant = NewTenant(); + var auth = Authorize(tenant); + var owner = Scope(tenant, team: "team-a"); + var recipient = Scope(tenant, team: "team-b"); + var record = await ValidatedAsync(auth, owner); + + await using var purging = await _fixture.DataSource.OpenConnectionAsync(); + await using var transaction = await purging.BeginTransactionAsync(); + await PurgeInAsync(purging, transaction, record.ExperienceId, tenant, owner.TeamId); + + // Issued while the purge is open, so it blocks on the record row the purge holds FOR UPDATE. + var issuing = _grants.CreateAsync( + auth, + new GrantAdministration(Administrator, DateTimeOffset.UtcNow), + new ExperienceGrantRequest( + Guid.NewGuid(), record.ExperienceId, owner, recipient, "a sibling team owns the follow-up", + DateTimeOffset.UtcNow.AddDays(90)), + CancellationToken.None); + + await Task.Delay(OverlapWindow); + await transaction.CommitAsync(); + + var issued = await issuing; + + Assert.Equal(ExperienceGrantOutcome.NotFound, issued.Outcome); + Assert.Equal(0, await CountAsync("experience_grants", record.ExperienceId)); + Assert.Equal(0, await CountGrantEventsAsync(record.ExperienceId)); + } + + [Fact] + public async Task Feedback_written_while_a_record_is_being_erased_loses_instead_of_surviving_the_purge() + { + // AD-1, the feedback path -- the one that leaves a reviewer identity and a free-text rationale + // about an erased record permanently in an append-only table. + var tenant = NewTenant(); + var auth = Authorize(tenant); + var scope = Scope(tenant); + var record = await ValidatedAsync(auth, scope); + + await using var purging = await _fixture.DataSource.OpenConnectionAsync(); + await using var transaction = await purging.BeginTransactionAsync(); + await PurgeInAsync(purging, transaction, record.ExperienceId, tenant, scope.TeamId); + + var feedback = Feedback(scope, [record.ExperienceId]); + var recording = _ledger.RecordAsync(auth, Submission(feedback), CancellationToken.None); + + await Task.Delay(OverlapWindow); + await transaction.CommitAsync(); + + var recorded = await recording; + + Assert.Equal(ExperienceReuseFeedbackStoreOutcome.Invalid, recorded.Outcome); + Assert.Equal("Exposures[0].ExperienceId", Assert.Single(recorded.Errors).Path); + Assert.Equal(0, await CountFeedbackAsync(feedback.FeedbackId)); + Assert.Equal(0, await CountAsync("reuse_feedback_exposures", record.ExperienceId)); + } + + [Fact] + public async Task Two_purges_sharing_one_submission_never_leave_it_orphaned() + { + // AD-4. A submission naming two records, both erased at once: each purge's "are there exposures + // left?" used to see the other's uncommitted delete and leave the parent, so the row survived + // describing nothing -- carrying a run ID, a scope, an outcome and a measure that nothing else + // would ever collect. The submissions are locked before their exposures are deleted now. + var tenant = NewTenant(); + var auth = Authorize(tenant); + var scope = Scope(tenant); + var first = await ValidatedAsync(auth, scope); + var second = await ValidatedAsync(auth, scope); + + var feedback = Feedback(scope, [first.ExperienceId, second.ExperienceId]); + Assert.Equal( + ExperienceReuseFeedbackOutcome.Recorded, + (await _feedback.RecordAsync(auth, feedback, CancellationToken.None)).Outcome); + + // Deterministically interleaved: the first purge holds its transaction open across the second's + // whole run, which is exactly the window the defect lived in. + await using (var purging = await _fixture.DataSource.OpenConnectionAsync()) + { + await using var transaction = await purging.BeginTransactionAsync(); + await PurgeInAsync(purging, transaction, first.ExperienceId, tenant, scope.TeamId); + + var concurrent = _store.DeleteAsync(auth, scope, second.ExperienceId, CancellationToken.None); + await Task.Delay(OverlapWindow); + await transaction.CommitAsync(); + + Assert.Equal(ExperienceStoreOutcome.Deleted, (await concurrent).Outcome); + } + + Assert.Equal(0, await CountFeedbackAsync(feedback.FeedbackId)); + Assert.Equal(0, await CountAsync("reuse_feedback_exposures", first.ExperienceId)); + Assert.Equal(0, await CountAsync("reuse_feedback_exposures", second.ExperienceId)); + + // ...and the same through two ordinary concurrent deletes, whatever order they happen to take. + var third = await ValidatedAsync(auth, scope); + var fourth = await ValidatedAsync(auth, scope); + var shared = Feedback(scope, [third.ExperienceId, fourth.ExperienceId]); + Assert.Equal( + ExperienceReuseFeedbackOutcome.Recorded, + (await _feedback.RecordAsync(auth, shared, CancellationToken.None)).Outcome); + + var raced = await Task.WhenAll( + _store.DeleteAsync(auth, scope, third.ExperienceId, CancellationToken.None), + _store.DeleteAsync(auth, scope, fourth.ExperienceId, CancellationToken.None)); + + Assert.All(raced, result => Assert.Equal(ExperienceStoreOutcome.Deleted, result.Outcome)); + Assert.Equal(0, await CountFeedbackAsync(shared.FeedbackId)); + } + + [Fact] + public async Task A_rolled_back_purge_leaves_a_fully_populated_record_exactly_as_it_was() + { + // Rollback was proven only on a record carrying nothing: no evidence, no exposures, no feedback, + // no grants, no embedding. Atomicity is worth exactly as much as the fullest record it holds for. + var tenant = NewTenant(); + var auth = Authorize(tenant); + var owner = Scope(tenant, team: "team-a"); + var recipient = Scope(tenant, team: "team-b"); + var record = await PopulatedAsync(auth, owner, recipient); + var feedbackId = Assert.Single(await FeedbackIdsAsync(record.ExperienceId)); + + var before = await ReadTombstoneAsync(record.ExperienceId); + var counts = await EveryCountAsync(record.ExperienceId, feedbackId); + + await using (var purging = await _fixture.DataSource.OpenConnectionAsync()) + { + await using var transaction = await purging.BeginTransactionAsync(); + await PurgeInAsync(purging, transaction, record.ExperienceId, tenant, owner.TeamId); + + // Inside the open transaction the erasure has really happened -- so the rollback below is + // undoing work, not asserting over a purge that never ran. + await using (var check = new NpgsqlCommand( + "SELECT count(*) FROM agent_experience.lifecycle_events WHERE experience_id = @id", purging, transaction)) + { + check.Parameters.Add(new NpgsqlParameter("id", record.ExperienceId)); + Assert.Equal(0L, await check.ExecuteScalarAsync()); + } + + await transaction.RollbackAsync(); + } + + Assert.Equal(before, await ReadTombstoneAsync(record.ExperienceId)); + Assert.Equal(counts, await EveryCountAsync(record.ExperienceId, feedbackId)); + Assert.Equal(ExperienceStoreOutcome.Found, (await _store.GetAsync(auth, owner, record.ExperienceId, CancellationToken.None)).Outcome); + } + + // ------------------------------------------------------------------ retention + + [Fact] + public async Task A_sweep_erases_only_what_a_frozen_clock_puts_past_the_cutoff_and_says_whether_more_remain() + { + var tenant = NewTenant(); + var auth = Authorize(tenant); + var scope = Scope(tenant); + var clock = new FrozenClock(ColumnTime); + var store = new PostgresExperienceRecordStore(_fixture.DataSource, onGrantsUnavailable: null, auditing: null, timeProvider: clock); + + // Four records at known ages, created oldest first, plus one that is comfortably inside the + // retention window. + var ancient = await SeedAtAsync(auth, scope, ColumnTime.AddDays(-400)); + var older = await SeedAtAsync(auth, scope, ColumnTime.AddDays(-200)); + var old = await SeedAtAsync(auth, scope, ColumnTime.AddDays(-100)); + var fresh = await SeedAtAsync(auth, scope, ColumnTime.AddDays(-1)); + + // A ninety-day retention, two at a time: the sweep is bounded and says another pass would find + // more, which is how a host's own scheduler drives it without this library owning a timer. + var first = await store.SweepExpiredAsync(auth, scope, TimeSpan.FromDays(90), 2, CancellationToken.None); + + Assert.Equal(ExperienceStoreOutcome.Deleted, first.Outcome); + Assert.Equal(2, first.DeletedCount); + Assert.True(first.MoreRemain); + + // Oldest first, so it is the two oldest that went. + Assert.NotNull((await ReadTombstoneAsync(ancient)).DeletedAt); + Assert.NotNull((await ReadTombstoneAsync(older)).DeletedAt); + Assert.Null((await ReadTombstoneAsync(old)).DeletedAt); + + var second = await store.SweepExpiredAsync(auth, scope, TimeSpan.FromDays(90), 2, CancellationToken.None); + + Assert.Equal(1, second.DeletedCount); + Assert.False(second.MoreRemain); + Assert.NotNull((await ReadTombstoneAsync(old)).DeletedAt); + + // The one inside the window is untouched, and a third pass finds nothing left to do. + Assert.Null((await ReadTombstoneAsync(fresh)).DeletedAt); + + var third = await store.SweepExpiredAsync(auth, scope, TimeSpan.FromDays(90), 2, CancellationToken.None); + Assert.Equal(0, third.DeletedCount); + Assert.False(third.MoreRemain); + + // Age is measured from CreatedAt on this store's own clock. Winding the clock forward by a year + // makes the record that was inside the window fall outside it -- and nothing else changed. + clock.Advance(TimeSpan.FromDays(365)); + + var later = await store.SweepExpiredAsync(auth, scope, TimeSpan.FromDays(90), 2, CancellationToken.None); + Assert.Equal(1, later.DeletedCount); + Assert.NotNull((await ReadTombstoneAsync(fresh)).DeletedAt); + + // And a sweep of a scope that holds nothing is a normal, empty answer rather than a refusal. + var elsewhere = await store.SweepExpiredAsync(auth, Scope(tenant, team: "team-empty"), TimeSpan.FromDays(1), 10, CancellationToken.None); + Assert.Equal(ExperienceStoreOutcome.Deleted, elsewhere.Outcome); + Assert.Equal(0, elsewhere.DeletedCount); + } + + [Fact] + public async Task A_sweep_never_reaches_another_scope() + { + var tenant = NewTenant(); + var auth = Authorize(tenant); + var mine = Scope(tenant, team: "team-a"); + var theirs = Scope(tenant, team: "team-b"); + var clock = new FrozenClock(ColumnTime); + var store = new PostgresExperienceRecordStore(_fixture.DataSource, onGrantsUnavailable: null, auditing: null, timeProvider: clock); + + var ours = await SeedAtAsync(auth, mine, ColumnTime.AddDays(-400)); + var other = await SeedAtAsync(auth, theirs, ColumnTime.AddDays(-400)); + + var swept = await store.SweepExpiredAsync(auth, mine, TimeSpan.FromDays(90), 50, CancellationToken.None); + + Assert.Equal(1, swept.DeletedCount); + Assert.NotNull((await ReadTombstoneAsync(ours)).DeletedAt); + Assert.Null((await ReadTombstoneAsync(other)).DeletedAt); + } + + [Fact] + public async Task Expired_grants_and_their_events_are_purged_and_a_live_grant_is_left_alone() + { + var tenant = NewTenant(); + var auth = Authorize(tenant); + var owner = Scope(tenant, team: "team-a"); + var recipient = Scope(tenant, team: "team-b"); + var other = Scope(tenant, team: "team-c"); + var record = await ValidatedAsync(auth, owner); + + var live = await IssueAsync(auth, record.ExperienceId, owner, recipient, DateTimeOffset.UtcNow.AddHours(1)); + + // A grant issued two days ago for one day, which is therefore a day past its expiry. It is + // seeded directly rather than issued and waited out: an expiry may only ever move closer + // (0006's monotonicity guard) but never behind issued_at (0005's CHECK), so the only honest way + // to have an expired grant is to have issued it in the past. + var expired = await SeedExpiredGrantAsync(record.ExperienceId, owner, other); + + var administration = new GrantAdministration(Administrator, DateTimeOffset.UtcNow); + var purged = await _grants.PurgeExpiredAsync(auth, administration, owner, 50, CancellationToken.None); + + Assert.Equal(ExperienceStoreOutcome.Deleted, purged.Outcome); + Assert.Equal(1, purged.PurgedCount); + Assert.False(purged.MoreRemain); + + // The expired grant and its whole trail are gone; the live one and its trail are untouched. + Assert.Equal(0, await CountGrantAsync(expired)); + Assert.Equal(0, await CountGrantEventsForAsync(expired)); + Assert.Equal(1, await CountGrantAsync(live)); + Assert.True(await CountGrantEventsForAsync(live) > 0); + + // Administrator authority is required, exactly as it is for issuing and revoking. + var unauthorized = await _grants.PurgeExpiredAsync(auth, administration: null, owner, 50, CancellationToken.None); + Assert.Equal(ExperienceStoreOutcome.Denied, unauthorized.Outcome); + + // And so is authorization over the owner scope itself. + var denied = await _grants.PurgeExpiredAsync(Authorize(NewTenant()), administration, owner, 50, CancellationToken.None); + Assert.Equal(ExperienceStoreOutcome.Denied, denied.Outcome); + + var invalid = await _grants.PurgeExpiredAsync(auth, administration, owner, 0, CancellationToken.None); + Assert.Equal(ExperienceStoreOutcome.Invalid, invalid.Outcome); + Assert.Equal("BatchSize", Assert.Single(invalid.Errors).Path); + } + + [Fact] + public async Task A_sweep_measures_age_on_created_at_even_when_the_record_has_been_written_since() + { + // The documented guarantee: "a record that is read, ranked, or re-scored does not thereby become + // younger". A fixture whose created_at and updated_at are equal cannot tell the two apart, so a + // sweep that measured the wrong one would pass -- and a repeatedly-reinforced record would never + // expire, which is a retention obligation silently unmet. + var tenant = NewTenant(); + var auth = Authorize(tenant); + var scope = Scope(tenant); + var clock = new FrozenClock(ColumnTime); + var store = new PostgresExperienceRecordStore(_fixture.DataSource, onGrantsUnavailable: null, auditing: null, timeProvider: clock); + + var stale = await SeedAtAsync(auth, scope, ColumnTime.AddDays(-400)); + + // A lifecycle commit stamps updated_at from the store's own clock, so this record is four hundred + // days old and was written a moment ago. + Assert.Equal( + ExperienceStoreOutcome.Committed, + (await store.CommitLifecycleEventAsync( + auth, scope, Event(stale, ExperienceStatus.Candidate, ExperienceStatus.Validated, 0), CancellationToken.None)).Outcome); + + var written = await ReadTombstoneAsync(stale); + Assert.True(written.UpdatedAt > written.CreatedAt, "The fixture has to separate the two timestamps or it proves nothing."); + Assert.Equal(ColumnTime, written.UpdatedAt); + + var swept = await store.SweepExpiredAsync(auth, scope, TimeSpan.FromDays(90), 50, CancellationToken.None); + + Assert.Equal(1, swept.DeletedCount); + Assert.NotNull((await ReadTombstoneAsync(stale)).DeletedAt); + } + + [Fact] + public async Task A_sweep_stamps_the_tombstone_from_the_injected_clock_and_says_so_when_it_stops_early() + { + var tenant = NewTenant(); + var auth = Authorize(tenant); + var scope = Scope(tenant); + var clock = new FrozenClock(ColumnTime); + var store = new PostgresExperienceRecordStore(_fixture.DataSource, onGrantsUnavailable: null, auditing: null, timeProvider: clock); + + // deleted_at is the injected clock's reading, to the microsecond, not merely "consistent with" + // created_at and updated_at. + var first = await SeedAtAsync(auth, scope, ColumnTime.AddDays(-400)); + var deleted = await store.DeleteAsync(auth, scope, first, CancellationToken.None); + + Assert.Equal(ExperienceStoreOutcome.Deleted, deleted.Outcome); + Assert.Equal(ColumnTime, (await ReadTombstoneAsync(first)).DeletedAt); + + // ...and the same clock stamps a sweep's tombstones. + var second = await SeedAtAsync(auth, scope, ColumnTime.AddDays(-399)); + var third = await SeedAtAsync(auth, scope, ColumnTime.AddDays(-398)); + + clock.Advance(TimeSpan.FromDays(3)); + + // Now the interruption. The sweep erases the older of the two and then blocks on the row another + // connection is holding, which is the moment the caller cancels -- so the count it reports is a + // fact about irreversible work rather than a number thrown away with the exception. + await using var holding = await _fixture.DataSource.OpenConnectionAsync(); + await using var hold = await holding.BeginTransactionAsync(); + await using (var pin = new NpgsqlCommand( + "SELECT revision FROM agent_experience.experience_records WHERE experience_id = @id FOR UPDATE", holding, hold)) + { + pin.Parameters.Add(new NpgsqlParameter("id", third)); + Assert.NotNull(await pin.ExecuteScalarAsync()); + } + + using var cancellation = new CancellationTokenSource(); + var sweeping = store.SweepExpiredAsync(auth, scope, TimeSpan.FromDays(90), 50, cancellation.Token); + + await Task.Delay(OverlapWindow); + await cancellation.CancelAsync(); + + var partial = await sweeping; + + Assert.True(partial.Interrupted); + Assert.Equal(1, partial.DeletedCount); + Assert.True(partial.MoreRemain); + Assert.Equal(ColumnTime.AddDays(3), (await ReadTombstoneAsync(second)).DeletedAt); + Assert.Null((await ReadTombstoneAsync(third)).DeletedAt); + + await hold.RollbackAsync(); + + // And the record it stopped on is still there to be erased by the next pass, which is what + // MoreRemain promised. + var resumed = await store.SweepExpiredAsync(auth, scope, TimeSpan.FromDays(90), 50, CancellationToken.None); + Assert.False(resumed.Interrupted); + Assert.Equal(1, resumed.DeletedCount); + } + + [Fact] + public async Task A_grant_purge_never_reaches_another_scope_and_bounds_its_own_batch() + { + var tenant = NewTenant(); + var auth = Authorize(tenant); + var mine = Scope(tenant, team: "team-a"); + var theirs = Scope(tenant, team: "team-b"); + var ours = await ValidatedAsync(auth, mine); + var other = await ValidatedAsync(auth, theirs); + + var expiredHere = await SeedExpiredGrantAsync(ours.ExperienceId, mine, Scope(tenant, team: "recipient-1")); + var expiredThere = await SeedExpiredGrantAsync(other.ExperienceId, theirs, Scope(tenant, team: "recipient-2")); + + var administration = new GrantAdministration(Administrator, DateTimeOffset.UtcNow); + var purged = await _grants.PurgeExpiredAsync(auth, administration, mine, 50, CancellationToken.None); + + // One scope's purge collects one scope's grants. Without the scope predicate this would erase + // every tenant's expired grants and their audit events, and nothing would have noticed. + Assert.Equal(1, purged.PurgedCount); + Assert.Equal(0, await CountGrantAsync(expiredHere)); + Assert.Equal(1, await CountGrantAsync(expiredThere)); + Assert.True(await CountGrantEventsForAsync(expiredThere) > 0); + + // The batch bound is the function's, not the caller's. Three more expired grants, two at a time. + var a = await SeedExpiredGrantAsync(ours.ExperienceId, mine, Scope(tenant, team: "recipient-3")); + var b = await SeedExpiredGrantAsync(ours.ExperienceId, mine, Scope(tenant, team: "recipient-4")); + var c = await SeedExpiredGrantAsync(ours.ExperienceId, mine, Scope(tenant, team: "recipient-5")); + + var page = await _grants.PurgeExpiredAsync(auth, administration, mine, 2, CancellationToken.None); + Assert.Equal(2, page.PurgedCount); + Assert.True(page.MoreRemain); + + var last = await _grants.PurgeExpiredAsync(auth, administration, mine, 2, CancellationToken.None); + Assert.Equal(1, last.PurgedCount); + Assert.False(last.MoreRemain); + Assert.Equal(0, await CountGrantAsync(a) + await CountGrantAsync(b) + await CountGrantAsync(c)); + + // And the bound holds for a hand-caller that never touches this adapter: LIMIT NULL means "no + // limit" in PostgreSQL, so a bound that lived only in the validator was no bound at all. + await SeedExpiredGrantsAsync(ours.ExperienceId, mine, PostgresExperienceRecordStore.MaxSweepBatchSize + 1); + + await using var command = _fixture.DataSource.CreateCommand( + "SELECT agent_experience.purge_expired_grants(@tenant, 'app-1', 'project-1', @team, NULL, NULL, now(), NULL)"); + command.Parameters.Add(new NpgsqlParameter("tenant", NpgsqlDbType.Text) { TypedValue = tenant }); + command.Parameters.Add(new NpgsqlParameter("team", NpgsqlDbType.Text) { TypedValue = mine.TeamId! }); + + Assert.Equal((long)PostgresExperienceRecordStore.MaxSweepBatchSize, await command.ExecuteScalarAsync()); + } + + [Fact] + public async Task A_grant_naming_an_erased_record_is_collected_and_a_skewed_host_clock_collects_nothing_extra() + { + var tenant = NewTenant(); + var auth = Authorize(tenant); + var owner = Scope(tenant, team: "team-a"); + var record = await ValidatedAsync(auth, owner); + + // A live grant, and a record erased out from under it. The record purge removes its grants in its + // own transaction, so this one is written afterwards -- the case the script justifies at length + // as "exactly the row nothing else would ever collect", and the one with no test at all. + Assert.Equal( + ExperienceStoreOutcome.Deleted, + (await _store.DeleteAsync(auth, owner, record.ExperienceId, CancellationToken.None)).Outcome); + + var orphan = await SeedGrantAsync(record.ExperienceId, owner, Scope(tenant, team: "recipient-1"), expiresIn: TimeSpan.FromDays(90)); + Assert.Equal(1, await CountGrantAsync(orphan)); + + // A second, live grant over a live record, to prove the collection is about the tombstone rather + // than about sweeping everything in the scope. + var live = await ValidatedAsync(auth, owner); + var kept = await SeedGrantAsync(live.ExperienceId, owner, Scope(tenant, team: "recipient-2"), expiresIn: TimeSpan.FromHours(1)); + + var administration = new GrantAdministration(Administrator, DateTimeOffset.UtcNow); + + // The host's clock is a day fast. That must not widen what is destroyed: the cutoff is + // LEAST(host, clock_timestamp()), so the grant that expires in an hour is still live. + var skewed = new PostgresExperienceGrantStore( + _fixture.DataSource, policy: null, timeProvider: new FrozenClock(DateTimeOffset.UtcNow.AddDays(1))); + + var purged = await skewed.PurgeExpiredAsync(auth, administration, owner, 50, CancellationToken.None); + + Assert.Equal(1, purged.PurgedCount); + Assert.Equal(0, await CountGrantAsync(orphan)); + Assert.Equal(1, await CountGrantAsync(kept)); + } + + // ------------------------------------------------------------------ what a tombstone still hides + + [Fact] + public async Task Another_scopes_tombstone_is_invisible_to_a_feedback_submission() + { + // The refusal names an exposure by position, so it is a statement about an ID the caller + // supplied. If the check could see another scope's tombstone, that refusal would tell one tenant + // that another tenant once held -- and erased -- a record under an ID it merely guessed. + var tenant = NewTenant(); + var auth = Authorize(tenant); + var mine = Scope(tenant, team: "team-a"); + var theirs = Scope(tenant, team: "team-b"); + var record = await ValidatedAsync(auth, theirs); + + Assert.Equal( + ExperienceStoreOutcome.Deleted, + (await _store.DeleteAsync(auth, theirs, record.ExperienceId, CancellationToken.None)).Outcome); + + var feedback = Feedback(mine, [record.ExperienceId]); + var recorded = await _ledger.RecordAsync(auth, Submission(feedback), CancellationToken.None); + + // Recorded, exactly as an ID that never existed anywhere would be: "the run saw an ID that + // resolves to nothing here" is a fact worth keeping, and it is the same fact either way. + Assert.Equal(ExperienceReuseFeedbackStoreOutcome.Recorded, recorded.Outcome); + Assert.Equal(1, await CountFeedbackAsync(feedback.FeedbackId)); + + // ...while the scope that owns the tombstone is still refused. + var owning = Feedback(theirs, [record.ExperienceId]); + var refused = await _ledger.RecordAsync(auth, Submission(owning), CancellationToken.None); + Assert.Equal(ExperienceReuseFeedbackStoreOutcome.Invalid, refused.Outcome); + Assert.Equal("Exposures[0].ExperienceId", Assert.Single(refused.Errors).Path); + } + + [Fact] + public async Task The_erased_text_itself_is_no_longer_findable_by_search() + { + // The point of erasing a record is that its words are gone, so the assertion has to be about the + // words -- searching for a term that only ever appeared in the erased payload. Searching for + // "deleted" instead would tokenize to the tombstone's own placeholder and prove the opposite of + // what it looks like it proves. + const string Distinctive = "zanzibarine"; + + var tenant = NewTenant(); + var auth = Authorize(tenant); + var scope = Scope(tenant); + var search = new PostgresExperienceCandidateSource(_fixture.DataSource); + var query = new ExperienceCandidateQuery( + scope, $"{Distinctive} reconciliation", [ExperienceStatus.Validated, ExperienceStatus.Reinforced], 0, 50); + + var record = Minimal(scope) with + { + TaskSummary = $"{Distinctive} reconciliation of a settlement ledger", + ReuseConfidence = 2d / 3d, + SupportingValidations = 1, + }; + + Assert.Equal(ExperienceStoreOutcome.Created, (await _store.CreateAsync(auth, record, CancellationToken.None)).Outcome); + Assert.Equal( + ExperienceStoreOutcome.Committed, + (await _store.CommitLifecycleEventAsync( + auth, scope, Event(record.ExperienceId, ExperienceStatus.Candidate, ExperienceStatus.Validated, 0), CancellationToken.None)).Outcome); + + // Findable by that word, which is what makes the assertion after the erasure mean something. + var before = await search.SearchAsync(auth, query, CancellationToken.None); + Assert.Contains(record.ExperienceId, before.Candidates.Select(c => c.Record.ExperienceId)); + + Assert.Equal( + ExperienceStoreOutcome.Deleted, + (await _store.DeleteAsync(auth, scope, record.ExperienceId, CancellationToken.None)).Outcome); + + var after = await search.SearchAsync(auth, query, CancellationToken.None); + Assert.Empty(after.Candidates); + + // Not filtered out of the answer -- gone from the generated index term itself, which is what + // "no separate index maintenance" rests on. + var tombstone = await ReadTombstoneAsync(record.ExperienceId); + Assert.Equal("'delet':1", tombstone.SearchVector); + Assert.DoesNotContain(Distinctive, tombstone.Payload, StringComparison.Ordinal); + Assert.DoesNotContain(Distinctive, tombstone.TaskId, StringComparison.Ordinal); + } + + [Fact] + public async Task Confidence_evidence_written_back_against_a_tombstone_is_never_read_as_a_replay() + { + // The evidence ID is a global primary key, and this is the one statement that looks a row up by + // it alone. Without the tombstone filter, a re-inserted ledger row naming an erased record would + // make a replayed submission come back Committed -- reporting a commit that never happened, and + // handing back the erased record's old revision and status on the way. + var tenant = NewTenant(); + var auth = Authorize(tenant); + var scope = Scope(tenant); + var record = await ValidatedAsync(auth, scope); + + var evidenceId = Guid.NewGuid(); + var eventId = Guid.NewGuid(); + var confidence = new ConfidenceUpdate( + EvidenceId: evidenceId, + Kind: ConfidenceEvidenceKind.Supporting, + Source: ConfidenceEvidenceSource.Machine, + RunId: Guid.NewGuid(), + VerificationRoundId: Guid.NewGuid(), + ReviewerIdentity: null, + RuleVersion: ReuseConfidenceHeuristic.RuleVersion, + PriorReuseConfidence: 2d / 3d, + NewReuseConfidence: 3d / 4d, + PriorSupportingValidations: 1, + NewSupportingValidations: 2, + PriorContradictions: 0, + NewContradictions: 0); + + var lifecycleEvent = Event(record.ExperienceId, ExperienceStatus.Validated, ExperienceStatus.Reinforced, 1, eventId) with + { + Confidence = confidence, + }; + + Assert.Equal( + ExperienceStoreOutcome.Committed, + (await _store.CommitLifecycleEventAsync(auth, scope, lifecycleEvent, CancellationToken.None)).Outcome); + + Assert.Equal( + ExperienceStoreOutcome.Deleted, + (await _store.DeleteAsync(auth, scope, record.ExperienceId, CancellationToken.None)).Outcome); + Assert.Equal(0, await CountAsync("confidence_evidence", record.ExperienceId)); + + // The ledger row put back by hand. Nothing in this library writes evidence against a tombstone -- + // the append-only guard refuses UPDATE and DELETE, not INSERT -- but nothing in the schema stops + // another tool from doing it either, which is exactly the case this filter exists for. + await RestoreEvidenceAsync(evidenceId, record.ExperienceId, eventId, confidence); + Assert.Equal(1, await CountAsync("confidence_evidence", record.ExperienceId)); + + // The identical submission again. Conflict, not Committed: the stored row's record is a + // tombstone, so the replay comparison never sees it and nothing about it leaks back. + var replay = await _store.CommitLifecycleEventAsync(auth, scope, lifecycleEvent, CancellationToken.None); + + Assert.Equal(ExperienceStoreOutcome.Conflict, replay.Outcome); + Assert.Null(replay.AppliedConfidence); + Assert.Equal(0, replay.Revision); + } + + [Fact] + public async Task The_feedback_store_stamps_recorded_at_from_its_own_injected_clock() + { + // The TimeProvider this story added to the feedback store is not exercised by any other test, so + // a store that ignored it entirely would look exactly as correct. + var tenant = NewTenant(); + var auth = Authorize(tenant); + var scope = Scope(tenant); + var record = await ValidatedAsync(auth, scope); + + var ledger = new PostgresExperienceReuseFeedbackStore(_fixture.DataSource, new FrozenClock(ColumnTime)); + var feedback = Feedback(scope, [record.ExperienceId]); + + Assert.Equal( + ExperienceReuseFeedbackStoreOutcome.Recorded, + (await ledger.RecordAsync(auth, Submission(feedback), CancellationToken.None)).Outcome); + + await using var command = _fixture.DataSource.CreateCommand( + "SELECT recorded_at, observed_at FROM agent_experience.reuse_feedback WHERE feedback_id = @id"); + command.Parameters.Add(new NpgsqlParameter("id", feedback.FeedbackId)); + + await using var reader = await command.ExecuteReaderAsync(); + Assert.True(await reader.ReadAsync()); + + // recorded_at is the store's own reading of when the row landed; observed_at is the caller's. + Assert.Equal(ColumnTime, reader.GetFieldValue(0)); + Assert.Equal(feedback.ObservedAt, reader.GetFieldValue(1)); + } + + // ------------------------------------------------------------------ helpers + + /// A record carrying one of everything an erasure has to reach. + private async Task PopulatedAsync(AuthorizationContext auth, Scope owner, Scope recipient) + { + var record = await ValidatedAsync(auth, owner); + + // Confidence evidence and a second lifecycle event. + await ApplyEvidenceAsync(auth, owner, record.ExperienceId); + + // A feedback submission and its exposure row. + Assert.Equal( + ExperienceReuseFeedbackOutcome.Recorded, + (await _feedback.RecordAsync(auth, Feedback(owner, [record.ExperienceId]), CancellationToken.None)).Outcome); + + // A grant, its issue event, and one recorded delivery through it. + await IssueAsync(auth, record.ExperienceId, owner, recipient, DateTimeOffset.UtcNow.AddHours(1)); + + var shared = await _store.GetAsync(auth, recipient, record.ExperienceId, CancellationToken.None); + Assert.Equal(ExperienceStoreOutcome.Found, shared.Outcome); + Assert.True(shared.SharedByGrant); + + return record with { Revision = 2 }; + } + + /// One counted piece of confidence evidence, which is also a second lifecycle event. + private async Task ApplyEvidenceAsync(AuthorizationContext auth, Scope scope, Guid experienceId) + { + var applied = await _lifecycle.ApplyEvidenceAsync( + auth, + new ApplyConfidenceEvidenceRequest( + EventId: Guid.NewGuid(), + ExperienceId: experienceId, + Scope: scope, + EvidenceId: Guid.NewGuid(), + Kind: ConfidenceEvidenceKind.Supporting, + Source: ConfidenceEvidenceSource.Machine, + RunId: Guid.NewGuid(), + VerificationRoundId: Guid.NewGuid(), + Reason: "reuse was observed to hold up", + Producer: "tests", + OccurredAt: PayloadTime), + CancellationToken.None); + + Assert.Equal(ConfidenceUpdateOutcome.Applied, applied.Outcome); + } + + private async Task IssueAsync( + AuthorizationContext auth, + Guid experienceId, + Scope owner, + Scope recipient, + DateTimeOffset expiry) + { + var result = await _grants.CreateAsync( + auth, + new GrantAdministration(Administrator, DateTimeOffset.UtcNow), + new ExperienceGrantRequest(Guid.NewGuid(), experienceId, owner, recipient, "a sibling team owns the follow-up", expiry), + CancellationToken.None); + + Assert.Equal(ExperienceGrantOutcome.Created, result.Outcome); + return result.Grant!.GrantId; + } + + /// + /// A grant written straight into the table, with an expiry the caller chooses. The library refuses to + /// issue one over a tombstone, so this is how a test stages the row the expired-grant purge exists to + /// collect: a permission naming a record that is already erased. + /// + private async Task SeedGrantAsync(Guid experienceId, Scope owner, Scope recipient, TimeSpan expiresIn) + { + var grantId = Guid.NewGuid(); + + await using var grant = _fixture.DataSource.CreateCommand( + "INSERT INTO agent_experience.experience_grants (grant_id, experience_id, tenant_id, application_id, " + + "project_id, team_id, agent_id, user_id, recipient_tenant_id, recipient_application_id, " + + "recipient_project_id, recipient_team_id, recipient_agent_id, recipient_user_id, reason, " + + "administrator_principal_id, issued_at, expires_at, revoked_at, revocation_reason) VALUES " + + "(@grant_id, @experience_id, @tenant, 'app-1', 'project-1', @team, NULL, NULL, @tenant, 'app-1', " + + "'project-1', @recipient_team, NULL, NULL, 'written outside this library', @administrator, " + + "now(), now() + @expires_in, NULL, NULL)"); + + grant.Parameters.Add(new NpgsqlParameter("grant_id", grantId)); + grant.Parameters.Add(new NpgsqlParameter("experience_id", experienceId)); + grant.Parameters.Add(new NpgsqlParameter("tenant", NpgsqlDbType.Text) { TypedValue = owner.TenantId }); + grant.Parameters.Add(new NpgsqlParameter("team", NpgsqlDbType.Text) { TypedValue = owner.TeamId! }); + grant.Parameters.Add(new NpgsqlParameter("recipient_team", NpgsqlDbType.Text) { TypedValue = recipient.TeamId! }); + grant.Parameters.Add(new NpgsqlParameter("administrator", NpgsqlDbType.Text) { TypedValue = Administrator }); + grant.Parameters.Add(new NpgsqlParameter("expires_in", expiresIn)); + + Assert.Equal(1, await grant.ExecuteNonQueryAsync()); + return grantId; + } + + /// + /// expired grants in one statement, each to its own recipient team, so a + /// hand-caller's unbounded batch has more than the function's maximum to reach for. + /// + private async Task SeedExpiredGrantsAsync(Guid experienceId, Scope owner, int count) + { + await using var grants = _fixture.DataSource.CreateCommand( + "INSERT INTO agent_experience.experience_grants (grant_id, experience_id, tenant_id, application_id, " + + "project_id, team_id, agent_id, user_id, recipient_tenant_id, recipient_application_id, " + + "recipient_project_id, recipient_team_id, recipient_agent_id, recipient_user_id, reason, " + + "administrator_principal_id, issued_at, expires_at, revoked_at, revocation_reason) " + + "SELECT gen_random_uuid(), @experience_id, @tenant, 'app-1', 'project-1', @team, NULL, NULL, @tenant, " + + "'app-1', 'project-1', 'bulk-' || i, NULL, NULL, 'a window that has since closed', @administrator, " + + "now() - interval '2 days', now() - interval '1 day', NULL, NULL FROM generate_series(1, @count) i"); + + grants.Parameters.Add(new NpgsqlParameter("experience_id", experienceId)); + grants.Parameters.Add(new NpgsqlParameter("tenant", NpgsqlDbType.Text) { TypedValue = owner.TenantId }); + grants.Parameters.Add(new NpgsqlParameter("team", NpgsqlDbType.Text) { TypedValue = owner.TeamId! }); + grants.Parameters.Add(new NpgsqlParameter("administrator", NpgsqlDbType.Text) { TypedValue = Administrator }); + grants.Parameters.Add(new NpgsqlParameter("count", count)); + + Assert.Equal(count, await grants.ExecuteNonQueryAsync()); + } + + /// + /// Puts one confidence-evidence row back after an erasure removed it, exactly as it was. The + /// append-only guard refuses UPDATE and DELETE, never INSERT, so this is reachable by any writer with + /// INSERT on the table -- which is the point. + /// + private async Task RestoreEvidenceAsync(Guid evidenceId, Guid experienceId, Guid eventId, ConfidenceUpdate confidence) + { + await using var command = _fixture.DataSource.CreateCommand( + "INSERT INTO agent_experience.confidence_evidence (evidence_id, experience_id, event_id, kind, source, " + + "run_id, verification_round_id, reviewer_identity, counted, actor, rule_version, detail, recorded_at, " + + "applied_revision, applied_status, prior_reuse_confidence, new_reuse_confidence, " + + "prior_supporting_validations, new_supporting_validations, prior_contradictions, new_contradictions) " + + "VALUES (@evidence_id, @experience_id, @event_id, @kind, @source, @run_id, @round_id, NULL, true, " + + "'tests', @rule_version, NULL, now(), 2, 'Reinforced', @prior_confidence, @new_confidence, " + + "@prior_supporting, @new_supporting, 0, 0)"); + + var parameters = command.Parameters; + parameters.Add(new NpgsqlParameter("evidence_id", evidenceId)); + parameters.Add(new NpgsqlParameter("experience_id", experienceId)); + parameters.Add(new NpgsqlParameter("event_id", eventId)); + parameters.Add(new NpgsqlParameter("kind", NpgsqlDbType.Text) { TypedValue = confidence.Kind.ToString() }); + parameters.Add(new NpgsqlParameter("source", NpgsqlDbType.Text) { TypedValue = confidence.Source.ToString() }); + parameters.Add(new NpgsqlParameter("run_id", confidence.RunId)); + parameters.Add(new NpgsqlParameter("round_id", confidence.VerificationRoundId!.Value)); + parameters.Add(new NpgsqlParameter("rule_version", NpgsqlDbType.Text) { TypedValue = confidence.RuleVersion }); + parameters.Add(new NpgsqlParameter("prior_confidence", confidence.PriorReuseConfidence)); + parameters.Add(new NpgsqlParameter("new_confidence", confidence.NewReuseConfidence)); + parameters.Add(new NpgsqlParameter("prior_supporting", confidence.PriorSupportingValidations)); + parameters.Add(new NpgsqlParameter("new_supporting", confidence.NewSupportingValidations)); + + Assert.Equal(1, await command.ExecuteNonQueryAsync()); + } + + /// A grant issued two days ago for one day: live when it was issued, expired now. + private async Task SeedExpiredGrantAsync(Guid experienceId, Scope owner, Scope recipient) + { + var grantId = Guid.NewGuid(); + + await using (var grant = _fixture.DataSource.CreateCommand( + "INSERT INTO agent_experience.experience_grants (grant_id, experience_id, tenant_id, application_id, " + + "project_id, team_id, agent_id, user_id, recipient_tenant_id, recipient_application_id, " + + "recipient_project_id, recipient_team_id, recipient_agent_id, recipient_user_id, reason, " + + "administrator_principal_id, issued_at, expires_at, revoked_at, revocation_reason) VALUES " + + "(@grant_id, @experience_id, @tenant, 'app-1', 'project-1', @team, NULL, NULL, @tenant, 'app-1', " + + "'project-1', @recipient_team, NULL, NULL, 'a window that has since closed', @administrator, " + + "now() - interval '2 days', now() - interval '1 day', NULL, NULL)")) + { + grant.Parameters.Add(new NpgsqlParameter("grant_id", grantId)); + grant.Parameters.Add(new NpgsqlParameter("experience_id", experienceId)); + grant.Parameters.Add(new NpgsqlParameter("tenant", NpgsqlDbType.Text) { TypedValue = owner.TenantId }); + grant.Parameters.Add(new NpgsqlParameter("team", NpgsqlDbType.Text) { TypedValue = owner.TeamId! }); + grant.Parameters.Add(new NpgsqlParameter("recipient_team", NpgsqlDbType.Text) { TypedValue = recipient.TeamId! }); + grant.Parameters.Add(new NpgsqlParameter("administrator", NpgsqlDbType.Text) { TypedValue = Administrator }); + Assert.Equal(1, await grant.ExecuteNonQueryAsync()); + } + + await using (var issued = _fixture.DataSource.CreateCommand( + "INSERT INTO agent_experience.experience_grant_events (event_id, grant_id, experience_id, action, " + + "tenant_id, application_id, project_id, team_id, agent_id, user_id, recipient_tenant_id, " + + "recipient_application_id, recipient_project_id, recipient_team_id, recipient_agent_id, " + + "recipient_user_id, reason, administrator_principal_id, administrator_authorized_at, expires_at, " + + "occurred_at, recorded_at) SELECT @event_id, g.grant_id, g.experience_id, 'Issued', g.tenant_id, " + + "g.application_id, g.project_id, g.team_id, g.agent_id, g.user_id, g.recipient_tenant_id, " + + "g.recipient_application_id, g.recipient_project_id, g.recipient_team_id, g.recipient_agent_id, " + + "g.recipient_user_id, g.reason, g.administrator_principal_id, g.issued_at, g.expires_at, " + + "g.issued_at, g.issued_at FROM agent_experience.experience_grants g WHERE g.grant_id = @grant_id")) + { + issued.Parameters.Add(new NpgsqlParameter("event_id", Guid.NewGuid())); + issued.Parameters.Add(new NpgsqlParameter("grant_id", grantId)); + Assert.Equal(1, await issued.ExecuteNonQueryAsync()); + } + + return grantId; + } + + private async Task ValidatedAsync(AuthorizationContext auth, Scope scope) + { + var record = Minimal(scope) with + { + ReuseConfidence = 2d / 3d, + SupportingValidations = 1, + Contradictions = 0, + }; + + Assert.Equal(ExperienceStoreOutcome.Created, (await _store.CreateAsync(auth, record, CancellationToken.None)).Outcome); + + var commit = await _store.CommitLifecycleEventAsync( + auth, scope, Event(record.ExperienceId, ExperienceStatus.Candidate, ExperienceStatus.Validated, 0), CancellationToken.None); + Assert.Equal(ExperienceStoreOutcome.Committed, commit.Outcome); + + return record with { Status = ExperienceStatus.Validated, Revision = 1 }; + } + + private async Task SeedAtAsync(AuthorizationContext auth, Scope scope, DateTimeOffset createdAt) + { + var record = Minimal(scope, createdAt: createdAt); + Assert.Equal(ExperienceStoreOutcome.Created, (await _store.CreateAsync(auth, record, CancellationToken.None)).Outcome); + return record.ExperienceId; + } + + private static ExperienceReuseFeedback Feedback(Scope scope, IReadOnlyList exposed) => new( + FeedbackId: Guid.NewGuid(), + RunId: Guid.NewGuid(), + Scope: scope, + ExposedExperienceIds: exposed, + RunOutcome: TaskVerificationStatus.Verified, + Measure: new("task-success", 1), + ObservedAt: ColumnTime, + TrialLabel: "memory-enabled"); + + /// The unattributed ledger shape of , for driving the port directly. + private static RecordedExperienceReuseFeedback Submission(ExperienceReuseFeedback feedback) => new( + feedback.FeedbackId, + feedback.RunId, + feedback.Scope, + feedback.RunOutcome, + feedback.ClaimedBenefit, + ExperienceReuseBenefit.Unknown, + ReuseAttributionSource.None, + ReviewerIdentity: null, + EvaluatorId: null, + VerificationRoundId: null, + AssessmentId: null, + Rationale: null, + EvidenceIds: [], + AttributedAt: null, + feedback.Measure, + feedback.TrialLabel, + feedback.ObservedAt, + [.. feedback.ExposedExperienceIds.Order().Select(id => new ExperienceReuseExposure(id, false, null))]); + + private async Task CountAsync(string table, Guid experienceId) + { + await using var command = _fixture.DataSource.CreateCommand( + $"SELECT count(*) FROM agent_experience.{table} WHERE experience_id = @id"); + command.Parameters.Add(new NpgsqlParameter("id", experienceId)); + return (long)(await command.ExecuteScalarAsync())!; + } + + private async Task CountGrantEventsAsync(Guid experienceId) => + await CountAsync("experience_grant_events", experienceId); + + private async Task CountGrantEventsForAsync(Guid grantId) => + await ScalarAsync("SELECT count(*) FROM agent_experience.experience_grant_events WHERE grant_id = @id", grantId); + + private async Task CountGrantAsync(Guid grantId) => + await ScalarAsync("SELECT count(*) FROM agent_experience.experience_grants WHERE grant_id = @id", grantId); + + private async Task CountFeedbackAsync(Guid feedbackId) => + await ScalarAsync("SELECT count(*) FROM agent_experience.reuse_feedback WHERE feedback_id = @id", feedbackId); + + private async Task ScalarAsync(string sql, Guid id) + { + await using var command = _fixture.DataSource.CreateCommand(sql); + command.Parameters.Add(new NpgsqlParameter("id", id)); + return (long)(await command.ExecuteScalarAsync())!; + } + + private async Task> FeedbackIdsAsync(Guid experienceId) + { + await using var command = _fixture.DataSource.CreateCommand( + "SELECT feedback_id FROM agent_experience.reuse_feedback_exposures WHERE experience_id = @id"); + command.Parameters.Add(new NpgsqlParameter("id", experienceId)); + + var ids = new List(); + await using var reader = await command.ExecuteReaderAsync(); + while (await reader.ReadAsync()) + { + ids.Add(reader.GetGuid(0)); + } + + return ids; + } + + /// The stored row, read column by column, because the point is what the columns hold. + private async Task ReadTombstoneAsync(Guid experienceId) + { + await using var command = _fixture.DataSource.CreateCommand( + "SELECT source_run_id, tenant_id, application_id, project_id, team_id, agent_id, user_id, task_id, status, " + + "reuse_confidence, supporting_validations, contradictions, revision, created_at, updated_at, payload::text, " + + "deleted_at, search_vector::text FROM agent_experience.experience_records WHERE experience_id = @id"); + command.Parameters.Add(new NpgsqlParameter("id", experienceId)); + + await using var reader = await command.ExecuteReaderAsync(); + Assert.True(await reader.ReadAsync(), "The record row is gone; a delete leaves a tombstone, never nothing."); + + return new StoredRow( + reader.GetGuid(0), + reader.GetString(1), + reader.GetString(2), + reader.GetString(3), + reader.IsDBNull(4) ? null : reader.GetString(4), + reader.IsDBNull(5) ? null : reader.GetString(5), + reader.IsDBNull(6) ? null : reader.GetString(6), + reader.GetString(7), + reader.GetString(8), + reader.GetDouble(9), + reader.GetInt32(10), + reader.GetInt32(11), + reader.GetInt64(12), + reader.GetFieldValue(13), + reader.GetFieldValue(14), + reader.GetString(15), + reader.IsDBNull(16) ? null : reader.GetFieldValue(16), + reader.GetString(17)); + } + + /// + /// Runs the purge function inside a transaction the caller keeps open, so a second writer can be + /// issued against a record that is erased but not yet committed. That window is where every one of + /// this story's concurrency defects lived, and holding it open is what makes a race a test rather + /// than a coincidence. + /// + private static async Task PurgeInAsync( + NpgsqlConnection connection, + NpgsqlTransaction transaction, + Guid experienceId, + string tenant, + string? team) + { + await using var purge = new NpgsqlCommand( + "SELECT purge_outcome FROM agent_experience.purge_experience_record(" + + "@id, @tenant, 'app-1', 'project-1', @team, NULL, NULL, NULL, now())", + connection, + transaction); + + purge.Parameters.Add(new NpgsqlParameter("id", experienceId)); + purge.Parameters.Add(new NpgsqlParameter("tenant", NpgsqlDbType.Text) { TypedValue = tenant }); + purge.Parameters.Add(new NpgsqlParameter("team", NpgsqlDbType.Text) { Value = (object?)team ?? DBNull.Value }); + + Assert.Equal("Deleted", await purge.ExecuteScalarAsync()); + } + + /// Runs one statement with the purge marker hand-set, which any session may do. + private async Task MarkedAsync(string sql, Guid id) + { + await using var connection = await _fixture.DataSource.OpenConnectionAsync(); + await using var transaction = await connection.BeginTransactionAsync(); + + await using (var marker = new NpgsqlCommand("SET LOCAL agent_experience.purge_authorized = 'on'", connection, transaction)) + { + await marker.ExecuteNonQueryAsync(); + } + + await using var command = new NpgsqlCommand(sql, connection, transaction); + command.Parameters.Add(new NpgsqlParameter("id", id)); + + try + { + var affected = await command.ExecuteNonQueryAsync(); + await transaction.CommitAsync(); + return affected; + } + catch + { + await transaction.RollbackAsync(CancellationToken.None); + throw; + } + } + + /// Every table an erasure sweeps, counted in one shape, so "nothing moved" is one assertion. + private async Task EveryCountAsync(Guid experienceId, Guid feedbackId) => new( + await CountAsync("lifecycle_events", experienceId), + await CountAsync("confidence_evidence", experienceId), + await CountAsync("reuse_feedback_exposures", experienceId), + await CountFeedbackAsync(feedbackId), + await CountAsync("experience_grants", experienceId), + await CountGrantEventsAsync(experienceId), + await CountAsync("experience_grant_access", experienceId)); + + private async Task ExecuteAsync(string sql, Guid? id, string? tenant = null) + { + await using var command = _fixture.DataSource.CreateCommand(sql); + if (id is { } value) + { + command.Parameters.Add(new NpgsqlParameter("id", value)); + } + + if (tenant is not null) + { + command.Parameters.Add(new NpgsqlParameter("tenant", NpgsqlDbType.Text) { TypedValue = tenant }); + } + + return await command.ExecuteNonQueryAsync(); + } + + private sealed record StoredCounts( + long LifecycleEvents, + long ConfidenceEvidence, + long Exposures, + long Feedback, + long Grants, + long GrantEvents, + long GrantAccess); + + private sealed record StoredRow( + Guid SourceRunId, + string TenantId, + string ApplicationId, + string ProjectId, + string? TeamId, + string? AgentId, + string? UserId, + string TaskId, + string Status, + double ReuseConfidence, + int SupportingValidations, + int Contradictions, + long Revision, + DateTimeOffset CreatedAt, + DateTimeOffset UpdatedAt, + string Payload, + DateTimeOffset? DeletedAt, + string SearchVector); + + /// A clock the test moves by hand, so "older than ninety days" is a fact rather than a wait. + private sealed class FrozenClock(DateTimeOffset now) : TimeProvider + { + private DateTimeOffset _now = now; + + public override DateTimeOffset GetUtcNow() => _now; + + public void Advance(TimeSpan by) => _now = _now.Add(by); + } +} diff --git a/tests/AgentExperience.Storage.Postgres.Tests/PostgresFinalizationTests.cs b/tests/AgentExperience.Storage.Postgres.Tests/PostgresFinalizationTests.cs index 3f4d1c6..6515eed 100644 --- a/tests/AgentExperience.Storage.Postgres.Tests/PostgresFinalizationTests.cs +++ b/tests/AgentExperience.Storage.Postgres.Tests/PostgresFinalizationTests.cs @@ -155,7 +155,7 @@ public async Task A_denied_storage_decision_leaves_no_record_and_no_event_for_th var records = await _store.QueryAsync(auth, new ExperienceRecordQuery(scope), CancellationToken.None); Assert.Empty(records.Records); - var history = await _store.GetFirstHistoryPageAsync(auth, scope, ExperienceFinalizationService.ExperienceIdFor(runId), CancellationToken.None); + var history = await _store.GetFirstHistoryPageAsync(auth, scope, ExperienceFinalizationService.ExperienceIdFor(runId, scope), CancellationToken.None); Assert.Equal(ExperienceStoreOutcome.NotFound, history.Outcome); } @@ -233,7 +233,7 @@ public async Task A_record_created_but_never_confirmed_stays_a_Candidate_and_a_r Assert.False(interrupted.IsDurable); // What is stored is a Candidate at revision 0 with no history -- never a reusable record. - var experienceId = ExperienceFinalizationService.ExperienceIdFor(runId); + var experienceId = ExperienceFinalizationService.ExperienceIdFor(runId, scope); var unconfirmed = (await _store.GetAsync(auth, scope, experienceId, CancellationToken.None)).Record!; Assert.Equal(ExperienceStatus.Candidate, unconfirmed.Status); Assert.Equal(0, unconfirmed.Revision); diff --git a/tests/AgentExperience.Storage.Postgres.Tests/PostgresFixture.cs b/tests/AgentExperience.Storage.Postgres.Tests/PostgresFixture.cs index 0e42f9b..4ebd7ca 100644 --- a/tests/AgentExperience.Storage.Postgres.Tests/PostgresFixture.cs +++ b/tests/AgentExperience.Storage.Postgres.Tests/PostgresFixture.cs @@ -26,6 +26,47 @@ public async Task InitializeAsync() await ExperienceSchemaMigrator.MigrateAsync(_dataSource, CancellationToken.None); } + /// + /// Creates a login role in the shared container with only the privileges + /// names, and returns a data source connecting as it. The caller owns the data source and disposes + /// it; the role goes away with the container. + /// + /// + /// For privilege tests -- proving that a role which is not the owner cannot reach an + /// operation, which the owning fixture connection can never prove about itself. + /// + /// A short name fragment: 1 to 20 lower-case ASCII letters, digits, or underscores. + /// Statements to run as the owner after the role exists, each naming the role as {role}. + public async Task CreateRoleAsync(string purpose, params string[] grants) + { + Assert.InRange(purpose.Length, 1, 20); + Assert.All(purpose, c => Assert.True(c is (>= 'a' and <= 'z') or (>= '0' and <= '9') or '_', $"Invalid purpose character '{c}'.")); + + var name = $"aen_{purpose}_{Guid.NewGuid():N}"; + const string Password = "aen-role-password"; + + // CREATE ROLE takes no parameters, so the identifier is interpolated; every character of it has + // just been checked against the allowlist above. + await using (var command = DataSource.CreateCommand($"CREATE ROLE \"{name}\" LOGIN PASSWORD '{Password}'")) + { + await command.ExecuteNonQueryAsync(); + } + + foreach (var grant in grants) + { + await using var command = DataSource.CreateCommand(grant.Replace("{role}", $"\"{name}\"", StringComparison.Ordinal)); + await command.ExecuteNonQueryAsync(); + } + + var builder = new NpgsqlConnectionStringBuilder(_container!.GetConnectionString()) + { + Username = name, + Password = Password, + }; + + return NpgsqlDataSource.Create(builder.ConnectionString); + } + /// /// Creates an empty database in the shared container and returns a data source for it. The caller /// owns the data source and disposes it; the database goes away with the container. diff --git a/tests/AgentExperience.Storage.Postgres.Vectors.Tests/PostgresDeindexingTests.cs b/tests/AgentExperience.Storage.Postgres.Vectors.Tests/PostgresDeindexingTests.cs index ed22f9d..cf66d63 100644 --- a/tests/AgentExperience.Storage.Postgres.Vectors.Tests/PostgresDeindexingTests.cs +++ b/tests/AgentExperience.Storage.Postgres.Vectors.Tests/PostgresDeindexingTests.cs @@ -192,6 +192,121 @@ public async Task A_removal_against_an_unreachable_index_never_fails_the_transit (await world.Indexing.RemoveAsync(world.Authorization, world.Scope, id, CancellationToken.None)).Outcome); } + [Fact] + public async Task Erasing_a_record_removes_its_embedding_and_the_tombstone_can_never_be_indexed_again() + { + // Story 4.5's eighth erasure step, which lives in the base package's purge function and has to + // reach a table the base package must not depend on. Here the vectors schema *is* applied, so + // the to_regclass guard finds it and the embedding goes with the record's payload. + var world = await TestWorld.CreateAsync(DataSource); + + var erased = await world.AddRecordAsync("token-refresh", "Refresh an expired token", "Refresh before expiry."); + var kept = await world.AddRecordAsync("cache-stampede", "Avoid a cache stampede", "Lock the refill."); + + Assert.Equal(ExperienceIndexingOutcome.Indexed, (await world.Indexing.IndexAsync(world.Authorization, world.Scope, erased)).Outcome); + Assert.Equal(ExperienceIndexingOutcome.Indexed, (await world.Indexing.IndexAsync(world.Authorization, world.Scope, kept)).Outcome); + + var deleted = await world.Store.DeleteAsync(world.Authorization, world.Scope, erased, CancellationToken.None); + + Assert.Equal(ExperienceStoreOutcome.Deleted, deleted.Outcome); + Assert.Equal(0L, await world.CountEmbeddingsAsync(erased)); + Assert.Equal(1L, await world.CountEmbeddingsAsync(kept)); + + // A write that was already in flight when the erasure landed is Missing, never Stale: there is + // no revision of an erased record that could ever be indexed, so there is nothing to retry. + var late = await world.Indexing.IndexAsync(world.Authorization, world.Scope, erased); + Assert.Equal(ExperienceIndexingOutcome.Missing, late.Outcome); + Assert.Equal(0L, await world.CountEmbeddingsAsync(erased)); + + // A re-index pass does not offer it either: the scan reads the summary and the lesson, and a + // tombstone has neither. + var reindexed = await world.Indexing.ReindexAsync( + world.Authorization, new ReindexExperienceRequest(world.Scope, null, Limit: 50), CancellationToken.None); + Assert.DoesNotContain(erased, reindexed.Records.Select(result => result.ExperienceId)); + Assert.Contains(kept, reindexed.Records.Select(result => result.ExperienceId)); + + // And neither retrieval channel returns it. + var retrieved = await world.Retrieval().RetrieveAsync( + new RetrieveExperienceRequest(world.Authorization, world.Scope, "Refresh an expired token"), + CancellationToken.None); + Assert.DoesNotContain(retrieved.Records, r => r.Record.ExperienceId == erased); + } + + [Fact] + public async Task A_write_naming_the_tombstone_s_own_revision_is_missing_rather_than_stale() + { + // The eligibility filters this table's reads already carry would hide a tombstone whatever the + // erasure predicates said -- a tombstone's status is a literal no ExperienceStatus names -- so a + // test that only searched would pass with the tombstone checks removed entirely. This one goes + // through the two statements that have no status filter at all: the conditional write, and the + // probe that explains why it wrote nothing. A write at the tombstone's *own* revision is the one + // a stale-revision guard cannot refuse on its own. + var world = await TestWorld.CreateAsync(DataSource); + + var id = await world.AddRecordAsync("token-refresh", "Refresh an expired token", "Refresh before expiry."); + var deleted = await world.Store.DeleteAsync(world.Authorization, world.Scope, id, CancellationToken.None); + + Assert.Equal(ExperienceStoreOutcome.Deleted, deleted.Outcome); + + var written = await world.Index.WriteAsync( + world.Authorization, + new ExperienceIndexWrite( + world.Scope, + id, + new ExperienceEmbeddingDescriptor("topic-embed-v1", 4, "hash", deleted.Revision), + TopicEmbeddingGenerator.VectorFor("Refresh an expired token")), + CancellationToken.None); + + // Missing, and specifically not Stale: Stale would name a revision to retry against, and there is + // no revision of an erased record that could ever be indexed. + Assert.Equal(ExperienceIndexOutcome.Missing, written.Outcome); + Assert.Equal(0, written.CurrentRevision); + Assert.Equal(0L, await world.CountEmbeddingsAsync(id)); + } + + [Fact] + public async Task An_embedding_written_while_a_record_is_being_erased_loses_instead_of_surviving_the_purge() + { + // The worst of the three concurrent-writer paths, because the row that survived would be a + // searchable derivative of exactly the summary and lesson the erasure was asked to destroy. The + // foreign key's own FOR KEY SHARE parks this writer against the purge and then releases it + // straight onto the tombstone; only a locking clause in the write's own SELECT makes it re-check. + var world = await TestWorld.CreateAsync(DataSource); + + var id = await world.AddRecordAsync("token-refresh", "Refresh an expired token", "Refresh before expiry."); + + await using var purging = await DataSource.OpenConnectionAsync(); + await using var transaction = await purging.BeginTransactionAsync(); + + await using (var purge = new NpgsqlCommand( + "SELECT purge_outcome FROM agent_experience.purge_experience_record(" + + "@id, @tenant, 'app-1', 'project-1', NULL, NULL, NULL, NULL, now())", + purging, + transaction)) + { + purge.Parameters.Add(new NpgsqlParameter("id", id)); + purge.Parameters.Add(new NpgsqlParameter("tenant", world.Scope.TenantId)); + Assert.Equal("Deleted", await purge.ExecuteScalarAsync()); + } + + // Issued while the purge holds the record row, so it parks rather than deciding against a + // snapshot the purge is about to invalidate. + var writing = world.Index.WriteAsync( + world.Authorization, + new ExperienceIndexWrite( + world.Scope, + id, + new ExperienceEmbeddingDescriptor("topic-embed-v1", 4, "hash", 0), + TopicEmbeddingGenerator.VectorFor("Refresh an expired token")), + CancellationToken.None); + + await Task.Delay(TimeSpan.FromMilliseconds(300)); + await transaction.CommitAsync(); + + Assert.Equal(ExperienceIndexOutcome.Missing, (await writing).Outcome); + Assert.Equal(0L, await world.CountEmbeddingsAsync(id)); + } + private static CommitLifecycleTransitionRequest Transition( TestWorld world, Guid experienceId, diff --git a/tests/AgentExperience.Storage.Postgres.Vectors.Tests/TestWorld.cs b/tests/AgentExperience.Storage.Postgres.Vectors.Tests/TestWorld.cs index 64bf61a..ed291d1 100644 --- a/tests/AgentExperience.Storage.Postgres.Vectors.Tests/TestWorld.cs +++ b/tests/AgentExperience.Storage.Postgres.Vectors.Tests/TestWorld.cs @@ -170,9 +170,43 @@ public async Task CountEmbeddingsAsync(Guid experienceId) => public Task BumpRevisionAsync(Guid experienceId, long revision) => ExecuteAsync("UPDATE agent_experience.experience_records SET revision = @revision WHERE experience_id = @id", experienceId, ("revision", revision)); - /// Deletes a record, to stage a write that lands after the record is gone. - public Task DeleteRecordAsync(Guid experienceId) => - ExecuteAsync("DELETE FROM agent_experience.experience_records WHERE experience_id = @id", experienceId); + /// + /// Makes a record row vanish outright, to stage a write that lands after the record is gone. + /// + /// Since 0010 this is not reachable by any supported path: the erasure leaves a tombstone + /// rather than removing the row, and experience_records_no_delete refuses a bare + /// DELETE from every session, marker or not, precisely because a freed experience_id + /// could be re-created with the old grants still applying to the new content. The table's owner can + /// still disable the guard, which is the escape hatch every guard in this schema has, and that is + /// what this helper does -- deliberately and visibly, so the tests that stage a vanished row are + /// staging something the schema now says should not happen, rather than something it permits. + /// + /// + public async Task DeleteRecordAsync(Guid experienceId) + { + await using var connection = await DataSource.OpenConnectionAsync(); + await using (var disable = new NpgsqlCommand( + "ALTER TABLE agent_experience.experience_records DISABLE TRIGGER experience_records_no_delete", + connection)) + { + await disable.ExecuteNonQueryAsync(); + } + + try + { + await using var delete = new NpgsqlCommand( + "DELETE FROM agent_experience.experience_records WHERE experience_id = @id", connection); + delete.Parameters.Add(new NpgsqlParameter("id", experienceId)); + await delete.ExecuteNonQueryAsync(); + } + finally + { + await using var enable = new NpgsqlCommand( + "ALTER TABLE agent_experience.experience_records ENABLE ALWAYS TRIGGER experience_records_no_delete", + connection); + await enable.ExecuteNonQueryAsync(); + } + } /// Rewrites the reflection's lesson in place, which is a content change the re-index has to notice. public Task RewriteLessonAsync(Guid experienceId, string lesson) =>