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) =>