Activity

1. polewskm

   added

   enhancementNew feature or requestNew feature or request

   follow-upWork a plan found and did not doWork a plan found and did not do

   on Oct 29, 2026
2. polewskm commented on Nov 2, 2026

   

   polewskm

   on Nov 2, 2026

   ContributorAuthor

   More actions

   Plan snapshot. The working copy is PLAN.md in the plan's checkout; this comment is not updated as steps run.

   Plan: feat: release BFF refresh leases by owner, pool provider responses (10 steps)

   Plan: feat: release BFF refresh leases by owner, pool provider responses

   Issue: #423 #422

   Goal

   The OpenID protocol client stops copying every provider response body, which may carry tokens, into unzeroed arrays:
   it streams the body under the size cap into a sensitive pooled buffer and parses JSON or decodes a JWT from it (#423).
   The BFF stops releasing a won refresh lease by elapsed time: the shared value set gains an atomic compare-and-remove,
   the lease stores a random owner token, and the winner always releases exactly its own lease when it finishes, so a lease
   another replica took after this one expired is never removed (#422).

   Context for the executor

   • Build: dotnet build NCode.Identity.slnx
   • Test: dotnet test tests/<Project>/<Project>.csproj --filter "FullyQualifiedName~<Name>" -- RunConfiguration.TreatNoTestsAsError=true
   • Gate (not a step; /ship-plan runs it): ./build/dod.ps1 -Fast -Affected
   • Conventions: read .github/instructions/csharp-production.instructions.md (src) and
     .github/instructions/csharp-testing.instructions.md (tests) before editing .cs files. Async members return
     ValueTask, end in Async, take the CancellationToken last. Never nest a call inside another call's arguments:
     hoist it into a named local. The null-forgiving ! fails the build in src/. Encode text only with
     NCode.Buffers.SecureEncoding.UTF8 (Encoding.UTF8 is banned, RS0030). Tests: Method_State_Expected names, one
     #region <Method> Tests per member, strict mocks with .Verifiable(), no // Arrange comments, blank lines between
     sections. Every new .cs file starts with the copyright preamble copied from a neighbor file.
   • Formatting: after editing C# files run dotnet csharpier format <each edited .cs file>; the gate runs
     dotnet csharpier check ..
   • PublicAPI files: keep entries sorted and LF line endings (git diff --stat shows the whole file changed when CRLF
     slipped in).
   • Gotchas:
     • Between steps 2 and 3 src/NCode.Identity.SharedState.Valkey does not compile (it lacks the new interface member),
       and between steps 4 and 6 tests/NCode.Identity.Bff.Tests does not compile. That is expected: each step's
       Done-when builds only what that step makes whole. Do not touch files a step does not list to "fix" it early.
     • Moq: a method returning ValueTask<BffRefreshLease?> is set up with .ReturnsAsync((BffRefreshLease?)value) and
       .ReturnsAsync((BffRefreshLease?)null); record-struct equality makes ReleaseRefreshLeaseAsync(Lease, ...) match.
     • JsonElement.Parse(ReadOnlySequence<byte>) is a C# 14 static extension member in NCode.Json (src/NCode.Json/JsonElements.cs);
       it resolves only with using NCode.Json;.

   Decisions

   • Read provider responses into pooled buffers #423: read the body by streaming HttpContent.ReadAsStreamAsync into BufferFactory.CreatePooledBufferWriter(isSensitive: true)
     under OpenIdClientOptions.MaxResponseSizeBytes, and drop LoadIntoBufferAsync. LoadIntoBufferAsync copies the
     body into HttpContent's own unzeroed buffer, so copying "the buffered content" into a pooled sequence would still
     leave a token copy behind; streaming keeps the only copy in the secure pool (ADR-1116 points 2–4). The cap is
     checked on Content-Length first and authoritatively while reading, as DefaultOutboundHttpContentReader does; the
     client cannot reference NCode.Identity.OpenId.Authentication (ADR-1016, ADR-1285), so the loop is a private helper.
   • Read provider responses into pooled buffers #423: a too-large body throws OpenIdProviderException("The response from '<uri>' is larger than <N> bytes.") from
     inside the try (no catch clause matches it); the HttpRequestException message becomes "could not be reached."
     Callers classify by exception type (BffProviderFailures), so nothing else changes.
   • Read provider responses into pooled buffers #423: JSON is parsed with NCode.Json's JsonElement.Parse(ReadOnlySequence<byte>); an application/jwt body is
     decoded with SecureEncoding.UTF8.GetString(ReadOnlySequence<byte>) (the BCL EncodingExtensions overload). The
     JWT stays a string because ProviderHttpResponse.Jwt is one. No ADR: this applies ADR-1116.
   • Release the BFF refresh lease by ownership #422: the shared-state primitive is a compare-and-remove by value bytes,
     IExpiringValueSet.TryRemoveAsync(string key, ReadOnlyMemory<byte> expectedValue, CancellationToken), not a
     remove-if-version. A remove-if-version needs versions that never repeat, which changes ADR-1267's version semantics
     for every value and needs a tombstone that outlives the entry or random start versions; a lease needs identity, not
     order. A random owner token as the value is the established lock-release pattern and keeps versions as they are.
   • Release the BFF refresh lease by ownership #422: the comparison ignores the version and uses SequenceEqual, not a constant-time compare: the owner token is
     not a secret an attacker can probe.
   • Release the BFF refresh lease by ownership #422: the store, not the caller, creates the owner token: DefaultBffSessionStore.TryAcquireRefreshLeaseAsync
     stores SecureEncoding.UTF8.GetBytes(BffKeyDerivation.CreateSecret()) (256 random bits, base64url) and returns a new
     public readonly record struct BffRefreshLease(string LeaseKey, string Owner), or null when the lease is held.
     ReleaseRefreshLeaseAsync(BffRefreshLease lease, CancellationToken) calls TryRemoveAsync with the owner's bytes.
   • Release the BFF refresh lease by ownership #422: DefaultBffUserTokenService always releases a won lease in its finally, with CancellationToken.None; the
     5-second margin and its acquiredAt/finishedAt arithmetic are removed. IsRefreshLeaseHeldAsync is unchanged.
   • Release the BFF refresh lease by ownership #422: custom IExpiringValueSet adapters and IBffSessionStore implementations must implement the new members; the
     family is pre-release (ADR-1025), so this is a clean cutover with no shim, marked Breaking: in the changelog.
   • Release the BFF refresh lease by ownership #422: the decision is recorded as ADR-1305, which amends ADR-1291 (point 2's release rule) and ADR-1267 (the port
     gains a member), following the amendment pattern of ADR-1274/ADR-1284. 1305 is the next free number on
     origin/main (0275962); if another change takes it before this one lands, renumber the file, title, index row and
     every link.

   Out of scope

   • Changing ExpiringValue versions or any other IExpiringValueSet operation.
   • DefaultOutboundHttpContentReader, RevocationDataFetcher and every other HTTP reader outside ProviderHttpClient.
   • BffAccessTokenHandler's request-content buffering.
   • Renaming BFF store metric operations (release_refresh_lease stays) or changing BffTokenOptions.RefreshLeaseDuration.
   • Catching a store failure during lease release (listed under Follow-ups).
   • Moving ADR citations out of comments (issue Move ADR citations out of /// doc comments #453).

   Steps

   [ ] 1. Stream provider responses into a pooled sensitive buffer

   • Issue: Read provider responses into pooled buffers #423
   • Files: src/NCode.Identity.OpenId.Client/Http/ProviderHttpClient.cs (edit), tests/NCode.Identity.OpenId.Client.Tests/Http/ProviderHttpClientTests.cs (edit)
   • Change: add using System.Buffers; and using NCode.Json;. In SendAsync(request, httpClientName, ct) replace the
     LoadIntoBufferAsync and ReadAsByteArrayAsync lines with: using var body = BufferFactory.CreatePooledBufferWriter(isSensitive: true);,
     var withinLimit = await TryReadBodyAsync(response.Content, body, token);, if (!withinLimit) throw new OpenIdProviderException($"The response from '{request.RequestUri}' is larger than {Options.MaxResponseSizeBytes} bytes.");
     and var content = body.AsReadOnlySequence;. Then bytes.Length == 0 becomes content.IsEmpty,
     SecureEncoding.UTF8.GetString(bytes).Trim() becomes SecureEncoding.UTF8.GetString(content).Trim(), and
     JsonElement.Parse(bytes) becomes JsonElement.Parse(content). Change the HttpRequestException message to
     $"The OpenID provider at '{request.RequestUri}' could not be reached.". Add
     private async ValueTask<bool> TryReadBodyAsync(HttpContent content, IBufferWriter<byte> destination, CancellationToken cancellationToken):
     return false when content.Headers.ContentLength > Options.MaxResponseSizeBytes; otherwise
     await using var stream = await content.ReadAsStreamAsync(cancellationToken); and loop: var memory = destination.GetMemory();,
     var read = await stream.ReadAsync(memory, cancellationToken);, return true when read == 0, return false when
     total + read > Options.MaxResponseSizeBytes (before Advance), else total += read; destination.Advance(read);.
     Extend the class <summary>: "It streams each response body into pooled memory that is zeroed when released,
     because a body may carry tokens."
   • Tests: rename SendAsync_WithBodyOverCap_Throws to SendAsync_WithContentLengthOverCap_ThrowsWithTheLimit and
     assert Assert.Contains("larger than 1024 bytes", exception.Message). Add
     SendAsync_WithBodyOverCapAndShortContentLength_ThrowsWithTheLimit (a StringContent of 2048 'a', "application/json",
     with content.Headers.ContentLength = 10, cap 1024, same assertion) and
     SendAsync_WithJsonLargerThanOneBufferSegment_ParsesJson (body {"issuer":"x","padding":"<100000 'a'>"} built with
     new string('a', 100_000); assert issuer is "x" and padding has length 100000).
   • Pattern: src/NCode.Identity.OpenId.Authentication/OutboundHttp/DefaultOutboundHttpContentReader.cs (the read loop);
     existing StubHttpMessageHandler tests in the same test file.
   • Done when: if (Select-String -Path src/NCode.Identity.OpenId.Client/Http/ProviderHttpClient.cs -Pattern 'ReadAsByteArrayAsync|LoadIntoBufferAsync' -Quiet) { exit 1 }; dotnet test tests/NCode.Identity.OpenId.Client.Tests/NCode.Identity.OpenId.Client.Tests.csproj --filter "FullyQualifiedName~ProviderHttpClientTests" -- RunConfiguration.TreatNoTestsAsError=true; if ($LASTEXITCODE) { exit 1 } else { exit 0 }

   [ ] 2. Add compare-and-remove to the value set port and its in-process adapter

   • Issue: Release the BFF refresh lease by ownership #422
   • Files: src/NCode.Identity.SharedState.Abstractions/IExpiringValueSet.cs (edit), src/NCode.Identity.SharedState.Abstractions/PublicAPI.Unshipped.txt (edit), src/NCode.Identity.SharedState/InProcess/DefaultInProcessExpiringValueSet.cs (edit), tests/NCode.Identity.SharedState.Tests/InProcess/DefaultInProcessExpiringValueSetTests.cs (edit)
   • Change: in IExpiringValueSet, between TryTakeAsync and RemoveAsync, add
     ValueTask<bool> TryRemoveAsync(string key, ReadOnlyMemory<byte> expectedValue, CancellationToken cancellationToken);
     documented: summary "Atomically removes the value named key only when its bytes equal expectedValue, so a caller
     removes only a value it wrote. Of any number of concurrent calls that name the same value, across every replica that
     shares the set, at most one succeeds."; expectedValue "The bytes the value must hold, compared byte for byte; the
     version is not compared."; returns "true if the value was removed; or false, leaving any value unchanged, if it is
     absent or expired or its bytes differ from expectedValue." (use <paramref>/<see langword> like the neighbors).
     Change the // See ADR-1267. comment to // See ADR-1267 and ADR-1305. Append to PublicAPI.Unshipped.txt:
     NCode.Identity.SharedState.IExpiringValueSet.TryRemoveAsync(string! key, System.ReadOnlyMemory<byte> expectedValue, System.Threading.CancellationToken cancellationToken) -> System.Threading.Tasks.ValueTask<bool>.
     In-process: ArgumentNullException.ThrowIfNull(key); var now = TimeProvider.GetUtcNow();; return false unless
     Entries.TryGetValue(key, out var existing), existing.ExpiresWhen > now and
     existing.Value.AsSpan().SequenceEqual(expectedValue.Span); else return Entries.TryRemove(KeyValuePair.Create(key, existing)).
   • Tests (#region TryRemoveAsync Tests): TryRemoveAsync_WhenValueMatches_RemovesItAndReturnsTrue,
     TryRemoveAsync_WhenValueDiffers_ReturnsFalseAndKeepsTheValue, TryRemoveAsync_WhenAbsent_ReturnsFalse,
     TryRemoveAsync_WhenExpiredAndAddedAgainWithAnotherValue_KeepsTheNewValue (add {1} expiring Start.AddSeconds(10),
     set Now = Start.AddSeconds(10), add {2}, TryRemoveAsync with {1} returns false, GetAsync still returns
     {2}), and TryRemoveAsync_WhenConcurrentForSameValue_ExactlyOneSucceeds (with RunConcurrentlyAsync).
   • Pattern: TryTakeAsync in the same class; the TryReplaceAsync and TryTakeAsync test regions.
   • Done when: dotnet test tests/NCode.Identity.SharedState.Tests/NCode.Identity.SharedState.Tests.csproj --filter "FullyQualifiedName~DefaultInProcessExpiringValueSetTests.TryRemoveAsync" -- RunConfiguration.TreatNoTestsAsError=true

   [ ] 3. Implement compare-and-remove in the Valkey adapter

   • Issue: Release the BFF refresh lease by ownership #422
   • Files: src/NCode.Identity.SharedState.Valkey/ValkeyExpiringValueSet.cs (edit), src/NCode.Identity.SharedState.Valkey/ValkeySharedStateMetrics.cs (edit), tests/NCode.Identity.SharedState.Valkey.Tests/ValkeyExpiringValueSetTests.cs (edit)
   • Change: in ValkeySharedStateMetrics, after ValueSetRemoveOperation, add
     internal const string ValueSetTryRemoveOperation = "value_set.try_remove";. In ValkeyExpiringValueSet, after
     ReplaceScript, add internal const string RemoveScript (doc: "The script that deletes KEYS[1] only when the bytes
     after its 8-byte version prefix equal ARGV[1]. It returns 1 when it deleted the value."):
     local current = redis.call('GET', KEYS[1]) / if not current or string.sub(current, 9) ~= ARGV[1] then return 0 end
     / redis.call('DEL', KEYS[1]) / return 1. Add TryRemoveAsync before RemoveAsync, shaped like TryReplaceAsync
     without the expiry check: var expected = expectedValue.ToArray();, measure ValueSetTryRemoveOperation, run
     database.ScriptEvaluateAsync(RemoveScript, [GetKey(key)], [expected]).WaitAsync(cancellationToken), return
     (long)result == 1. Add to the class <summary>: "A compare-and-remove is a second script that compares the bytes
     after the version prefix and, only when they match, runs DEL."
   • Tests (#region TryRemoveAsync Tests, add a SetupRemove(RedisResult result) helper like SetupReplace with
     values [Payload]): TryRemoveAsync_RunsTheScriptWithTheValueAndReturnsTrue (asserts true and
     AssertOperationRecorded(ValkeySharedStateMetrics.ValueSetTryRemoveOperation, errorType: null)) and
     TryRemoveAsync_WhenScriptReturnsZero_ReturnsFalse.
   • Pattern: TryReplaceAsync and SetupReplace in the same files.
   • Done when: dotnet test tests/NCode.Identity.SharedState.Valkey.Tests/NCode.Identity.SharedState.Valkey.Tests.csproj --filter "FullyQualifiedName~ValkeyExpiringValueSetTests.TryRemoveAsync" -- RunConfiguration.TreatNoTestsAsError=true

   [ ] 4. Give the BFF lease contract an owner token

   • Issue: Release the BFF refresh lease by ownership #422
   • Files: src/NCode.Identity.Bff.Abstractions/BffRefreshLease.cs (new), src/NCode.Identity.Bff.Abstractions/IBffSessionStore.cs (edit), src/NCode.Identity.Bff.Abstractions/PublicAPI.Unshipped.txt (edit)
   • Change: new [PublicAPI] public readonly record struct BffRefreshLease(string LeaseKey, string Owner); in namespace
     NCode.Identity.Bff, with // See ADR-1305. above it. Summary: "A refresh lease that an IBffSessionStore granted:
     the key it guards and the random owner token that only its holder knows, so a release removes only this hold."
     <param name="LeaseKey">The key of the lease.</param>, <param name="Owner">The owner token the store recorded when it granted the lease.</param>. In IBffSessionStore: TryAcquireRefreshLeaseAsync returns
     ValueTask<BffRefreshLease?>, summary "Acquires a refresh lease under a new owner token when none is held under the
     key.", returns "The acquired lease, which names its owner; or null if the lease is held." ReleaseRefreshLeaseAsync
     becomes ValueTask ReleaseRefreshLeaseAsync(BffRefreshLease lease, CancellationToken cancellationToken), summary
     "Releases a refresh lease only while the owner that acquired it still holds it. Releasing a lease that expired, that
     another owner holds, or that is not held does nothing.", <param name="lease">The lease that TryAcquireRefreshLeaseAsync returned.</param>. In PublicAPI.Unshipped.txt replace the two changed
     IBffSessionStore lines with the new signatures
     (...ReleaseRefreshLeaseAsync(NCode.Identity.Bff.BffRefreshLease lease, System.Threading.CancellationToken cancellationToken) -> System.Threading.Tasks.ValueTask,
     ...TryAcquireRefreshLeaseAsync(string! leaseKey, System.DateTimeOffset expiresWhen, System.Threading.CancellationToken cancellationToken) -> System.Threading.Tasks.ValueTask<NCode.Identity.Bff.BffRefreshLease?>)
     and add, sorted: NCode.Identity.Bff.BffRefreshLease, .BffRefreshLease() -> void,
     .BffRefreshLease(string! LeaseKey, string! Owner) -> void, .LeaseKey.get -> string!, .LeaseKey.init -> void,
     .Owner.get -> string!, .Owner.init -> void (each prefixed NCode.Identity.Bff.BffRefreshLease).
   • Pattern: src/NCode.Identity.SharedState.Abstractions/ExpiringValue.cs and its PublicAPI lines; BffSessionRecord.cs for the preamble and namespace.
   • Done when: if (-not (Select-String -Path src/NCode.Identity.Bff.Abstractions/PublicAPI.Unshipped.txt -SimpleMatch 'ValueTask<NCode.Identity.Bff.BffRefreshLease?>' -Quiet)) { exit 1 }; dotnet build src/NCode.Identity.Bff.Abstractions/NCode.Identity.Bff.Abstractions.csproj --nologo; if ($LASTEXITCODE) { exit 1 } else { exit 0 }

   [ ] 5. Acquire and release the lease by its owner in the BFF

   • Issue: Release the BFF refresh lease by ownership #422
   • Files: src/NCode.Identity.Bff/Sessions/DefaultBffSessionStore.cs (edit), src/NCode.Identity.Bff/Sessions/BffInstrumentedSessionStore.cs (edit), src/NCode.Identity.Bff/Tokens/DefaultBffUserTokenService.cs (edit)
   • Change: DefaultBffSessionStore (add using NCode.Buffers;): TryAcquireRefreshLeaseAsync returns
     ValueTask<BffRefreshLease?>; inside the existing try: var key = BffStoreKeys.Lease(leaseKey);,
     var owner = BffKeyDerivation.CreateSecret();, var value = SecureEncoding.UTF8.GetBytes(owner);,
     var added = await values.TryAddAsync(key, value, expiresWhen, cancellationToken);,
     return added ? new BffRefreshLease(leaseKey, owner) : null;. ReleaseRefreshLeaseAsync(BffRefreshLease lease, ...):
     var key = BffStoreKeys.Lease(lease.LeaseKey);, var owner = SecureEncoding.UTF8.GetBytes(lease.Owner);,
     await values.TryRemoveAsync(key, owner, cancellationToken);. Keep the catch ... when (IsStoreFailure(...)) wrappers.
     BffInstrumentedSessionStore: same two signature changes, passing through to _inner under the same metric names.
     DefaultBffUserTokenService.AcquireAndRefreshAsync: delete acquiredAt; var leaseExpiresAt = now + tokenOptions.RefreshLeaseDuration;;
     var acquired = await _sessionStore.TryAcquireRefreshLeaseAsync(leaseKey, leaseExpiresAt, cancellationToken);
if (acquired is not { } lease) return null;. Replace the whole finally body with the comment
     // releases only this hold: a lease another replica took after this one expired has another owner and stays
     and await _sessionStore.ReleaseRefreshLeaseAsync(lease, CancellationToken.None);.
   • Pattern: the existing lease methods in the same files.
   • Done when: if (Select-String -Path src/NCode.Identity.Bff/Tokens/DefaultBffUserTokenService.cs -Pattern 'safetyMargin|acquiredAt' -Quiet) { exit 1 }; dotnet build src/NCode.Identity.Bff/NCode.Identity.Bff.csproj --nologo; if ($LASTEXITCODE) { exit 1 } else { exit 0 }

   [ ] 6. Cover the owner-token lease in the BFF tests

   • Issue: Release the BFF refresh lease by ownership #422
   • Files: tests/NCode.Identity.Bff.Tests/Sessions/DefaultBffSessionStoreTests.cs (edit), tests/NCode.Identity.Bff.Tests/Sessions/BffInstrumentedSessionStoreTests.cs (edit), tests/NCode.Identity.Bff.Tests/Tokens/DefaultBffUserTokenServiceTests.cs (edit)
   • Change: DefaultBffSessionStoreTests Lease Tests region: replace TryAcquireRefreshLeaseAsync_Called_AddsTheMarker
     with TryAcquireRefreshLeaseAsync_WhenAdded_ReturnsTheLeaseWithTheStoredOwner (capture the TryAddAsync value with
     .Callback; assert LeaseKey == "l1", Owner.Length == 43, captured bytes equal SecureEncoding.UTF8.GetBytes(Owner)),
     add TryAcquireRefreshLeaseAsync_WhenHeld_ReturnsNull and TryAcquireRefreshLeaseAsync_CalledTwice_UsesDistinctOwners;
     replace ReleaseRefreshLeaseAsync_Called_RemovesTheLease with ReleaseRefreshLeaseAsync_Called_RemovesOnlyTheOwnersValue
     (declare var expected = "owner-1"u8.ToArray(); first, because a u8 literal cannot appear inside the expression
     tree; strict setup TryRemoveAsync("bff:lease:l1", It.Is<ReadOnlyMemory<byte>>(v => v.ToArray().SequenceEqual(expected)), CancellationToken.None)
     returning true; call with new BffRefreshLease("l1", "owner-1")). IsRefreshLeaseHeldAsync tests may keep 0x31.
     BffInstrumentedSessionStoreTests: the release test passes new BffRefreshLease("lease-key", "owner-1") in setup and call.
     DefaultBffUserTokenServiceTests: add private static readonly BffRefreshLease Lease = new(LeaseKey, "owner-1");;
     SetupLeaseAcquired and the inline acquire setup in GetAccessTokenAsync_CallerCancelsAfterWinningLease_StillWritesRotatedTokens
     return (BffRefreshLease?)Lease; SetupLeaseLost returns (BffRefreshLease?)null; every
     ReleaseRefreshLeaseAsync(LeaseKey, CancellationToken.None) setup becomes ReleaseRefreshLeaseAsync(Lease, CancellationToken.None).
     In GetAccessTokenAsync_LostLeaseThenChangedRefreshToken_RefreshesWithNewToken the changed-key acquire returns
     (BffRefreshLease?)new BffRefreshLease(changedLeaseKey, "owner-2") and add a verifiable release setup for that lease.
     Replace GetAccessTokenAsync_RefreshSlowerThanMargin_KeepsLease with
     GetAccessTokenAsync_RefreshOutlastsTheLease_StillReleasesItsOwnLease: same arrangement but the token call sets
     TimeProvider.UtcNow = Now.AddSeconds(41), a release setup for Lease, and
     MockStore.Verify(x => x.ReleaseRefreshLeaseAsync(Lease, CancellationToken.None), Times.Once).
   • Pattern: the existing tests in the same files.
   • Done when: dotnet test tests/NCode.Identity.Bff.Tests/NCode.Identity.Bff.Tests.csproj --filter "FullyQualifiedName~DefaultBffSessionStoreTests|FullyQualifiedName~BffInstrumentedSessionStoreTests|FullyQualifiedName~DefaultBffUserTokenServiceTests" -- RunConfiguration.TreatNoTestsAsError=true

   [ ] 7. Record the owner-token release in ADR-1305

   • Issue: Release the BFF refresh lease by ownership #422
   • Files: docs/decisions/1305-a-refresh-lease-is-released-only-by-the-owner-token-its-winner-holds.md (new), docs/decisions/README.md (edit)
   • Change: copy the shape of docs/decisions/0000-template.md. Title # 1305. A refresh lease is released only by the owner token its winner holds; Status Accepted; Date 2026-11-01; Deciders NCode Group. Timeless present tense.
     Context: ADR-1291 point 2 releases a won lease only when the winner finished more than 5 s before the lease expires,
     because IExpiringValueSet (ADR-1267) has no compare-and-remove and a re-added key restarts at version 1 (ABA); a
     slow winner therefore leaves its lease held until it expires, and the margin is a guess about clocks. Decision:
     (1) IExpiringValueSet.TryRemoveAsync(key, expectedValue, ct) removes the live value only while its bytes equal
     expectedValue, ignoring the version; the in-process adapter removes the exact entry it compared, and the Valkey
     adapter runs one script that compares the bytes after the 8-byte version prefix and runs DEL, recorded as
     value_set.try_remove. (2) IBffSessionStore.TryAcquireRefreshLeaseAsync stores a fresh 256-bit random owner token
     as the lease value and returns BffRefreshLease(LeaseKey, Owner), or null when held;
     ReleaseRefreshLeaseAsync(BffRefreshLease) compare-and-removes with that owner. (3) The winner always releases its
     lease when the refresh ends, in a finally with CancellationToken.None; the time margin is gone. Options
     considered (each with why rejected): the 5-second time margin; remove-if-version with versions that never repeat
     (changes every value's version semantics, needs tombstones or random start versions, and a lease needs identity not
     order); a read-then-DEL without a script (not atomic); a server-side conditional-delete command (not every server
     the adapter supports has one, and the stored string carries the version prefix). Consequences: a winner frees its
     lease as soon as it finishes, so losers wait only for the refresh; a lease another replica took is never removed;
     custom IExpiringValueSet adapters and IBffSessionStore implementations implement the new members; the script is
     covered by mock-based tests only, as the other scripts are. References: ADR-1267, ADR-1291, issue Release the BFF refresh lease by ownership #422,
     IExpiringValueSet.cs, DefaultInProcessExpiringValueSet.cs, ValkeyExpiringValueSet.cs, DefaultBffSessionStore.cs,
     DefaultBffUserTokenService.cs. Append to the README index, after the 1304 row:
     | [1305](./1305-a-refresh-lease-is-released-only-by-the-owner-token-its-winner-holds.md) | A refresh lease is released only by the owner token its winner holds | Accepted |.
   • Pattern: docs/decisions/1284-a-logout-token-is-recorded-only-after-the-client-handles-it.md (an amending ADR).
   • Done when: $problems = @(& ./build/adr-index.ps1); if ($problems.Count -or -not (Select-String -Path docs/decisions/README.md -SimpleMatch '[1305](./1305-a-refresh-lease-is-released-only-by-the-owner-token-its-winner-holds.md)' -Quiet)) { $problems; exit 1 } else { exit 0 }

   [ ] 8. Mark ADR-1267 and ADR-1291 as amended by ADR-1305

   • Issue: Release the BFF refresh lease by ownership #422
   • Files: docs/decisions/1267-replicas-share-versioned-expiring-values-and-membership-indexes.md (edit), docs/decisions/1291-the-bff-refreshes-a-session-single-flight-under-a-lease-on-its-refresh-token.md (edit), docs/decisions/README.md (edit)
   • Change: change only the Status lines (keep the decision text). ADR-1267: - **Status:** Accepted (amended by [ADR-1305](1305-a-refresh-lease-is-released-only-by-the-owner-token-its-winner-holds.md): the value port gains TryRemoveAsync, an atomic compare-and-remove by value). ADR-1291: - **Status:** Accepted (amended by [ADR-1305](1305-a-refresh-lease-is-released-only-by-the-owner-token-its-winner-holds.md): the winner always releases its lease by the owner token it holds, instead of only when it finished 5 s before the lease expires). Wrap at 120
     columns with two-space continuation indent, as ADR-1274's Status line does. In the README index set the Status cell
     of rows 1267 and 1291 to Accepted (amended by [1305](./1305-a-refresh-lease-is-released-only-by-the-owner-token-its-winner-holds.md)).
   • Pattern: the Status line of docs/decisions/1274-the-protocol-client-validates-logout-tokens-and-rejects-replays.md and its README row.
   • Done when: $a = Select-String -Path docs/decisions/1267-*.md, docs/decisions/1291-*.md -SimpleMatch '1305-a-refresh-lease' -List; $r = Select-String -Path docs/decisions/README.md -SimpleMatch 'Accepted (amended by [1305]'; $p = @(& ./build/adr-index.ps1); if (@($a).Count -eq 2 -and @($r).Count -eq 2 -and -not $p.Count) { exit 0 } else { exit 1 }

   [ ] 9. Update the BFF design and metrics docs

   • Issue: Release the BFF refresh lease by ownership #422
   • Files: docs/backend-for-frontend.md (edit), docs/metrics.md (edit)
   • Change: in docs/backend-for-frontend.md §5.4, in the Versioned expiring values bullet, after "TryTakeAsync
     reads and removes in one atomic step." add "TryRemoveAsync removes a value only while it still holds the bytes the
     caller expects." and change "a compare-and-swap script" to "compare-and-swap and compare-and-remove scripts". In §5.5
     step 2, after the lease description add the sentence: "The lease's value is a random owner token, and
     ReleaseRefreshLeaseAsync removes the lease only while that owner holds it
     (ADR-1305)." In step 3,
     after the first sentence add "It then releases its own lease, however long the refresh took." In the mermaid
     diagram, after F->>S: compare-and-swap the session with the new tokens add
     F->>L: release the lease, only while this replica's owner token holds it. In docs/metrics.md,
     in the ncode.identity.shared_state.operation row insert `value_set.try_remove`, after `value_set.remove`,
     and widen the header and separator cells of that table by the same number of characters so the columns stay aligned.
   • Pattern: surrounding prose in the same sections.
   • Done when: if ((Select-String -Path docs/backend-for-frontend.md -SimpleMatch 'TryRemoveAsync' -Quiet) -and (Select-String -Path docs/backend-for-frontend.md -SimpleMatch '1305-a-refresh-lease' -Quiet) -and (Select-String -Path docs/metrics.md -SimpleMatch 'value_set.try_remove' -Quiet)) { exit 0 } else { exit 1 }

   [ ] 10. Record both changes in the changelog

   • Issue: Read provider responses into pooled buffers #423 Release the BFF refresh lease by ownership #422
   • Files: CHANGELOG.md (edit)
   • Change: in ## [Unreleased] (the first such heading), insert as the first entry under its existing ### Changed
     heading (the first ### Changed in the file), followed by a blank line:

     - **Breaking: the BFF releases a refresh lease only by the owner token its winner holds.** `IExpiringValueSet` gains
       `TryRemoveAsync(key, expectedValue, cancellationToken)`, an atomic compare-and-remove that deletes a value only while
       it still holds the expected bytes; the in-process and Valkey adapters implement it, and the Valkey script is
       recorded as the `value_set.try_remove` operation. `IBffSessionStore.TryAcquireRefreshLeaseAsync` returns a
       `BffRefreshLease` that names a fresh random owner token, or `null` when the lease is held, and
       `ReleaseRefreshLeaseAsync` takes that lease and removes it only while its owner still holds it. The winner of a
       refresh therefore always releases its own lease as soon as it finishes, instead of only when it finished 5 seconds
       before the lease expired, and never removes a lease another replica took after its own expired. A custom
       `IExpiringValueSet` adapter or `IBffSessionStore` implements the new members
       ([#422](https://github.com/NCodeGroup/Identity/issues/422),
       [ADR-1305](docs/decisions/1305-a-refresh-lease-is-released-only-by-the-owner-token-its-winner-holds.md)).
     

     and as the first entry under its existing ### Security heading (the first ### Security in the file), followed by a blank line:

     - **The OpenID protocol client keeps provider responses in zeroed, pooled memory.** It streams each response body,
       under `OpenIdClientOptions.MaxResponseSizeBytes`, into a sensitive pooled buffer that is zeroed when released,
       instead of copying it into `HttpContent`'s buffer and then a fresh `byte[]`, because a body may carry tokens. It
       parses JSON from that buffer with `JsonElement.Parse(ReadOnlySequence<byte>)` and decodes an `application/jwt`
       body from it. A body larger than the limit, whatever its `Content-Length` says, fails with an
       `OpenIdProviderException` that names the limit ([#423](https://github.com/NCodeGroup/Identity/issues/423)).
     

     Add no new ### heading.
   • Pattern: the existing entries under the same headings.
   • Done when: if ((Select-String -Path CHANGELOG.md -SimpleMatch 'the BFF releases a refresh lease only by the owner token its winner holds' -Quiet) -and (Select-String -Path CHANGELOG.md -SimpleMatch 'keeps provider responses in zeroed, pooled memory' -Quiet) -and @(Select-String -Path CHANGELOG.md -Pattern '^### Changed$').Count -eq @(git show HEAD:CHANGELOG.md | Select-String -Pattern '^### Changed$').Count) { exit 0 } else { exit 1 }

   Review checklist

   • ProviderHttpClient holds no byte[] copy of the body; the Content-Length early reject and the streaming cap
     both throw the "larger than N bytes" OpenIdProviderException; a timeout during the body read still maps to the
     timeout exception (the read uses the linked timeout token).
   • The Valkey RemoveScript compares string.sub(current, 9) (the bytes after the 8-byte big-endian version) with the
     raw expected bytes, and TryRemoveAsync passes the unencoded value, not Encode(...).
   • DefaultBffUserTokenService releases a won lease on every path, success and failure, with CancellationToken.None,
     and never releases a lease it did not win.
   • The cross-replica end-to-end test still passes:
     dotnet test tests/NCode.Identity.OpenId.Integration.Tests/NCode.Identity.OpenId.Integration.Tests.csproj --filter "FullyQualifiedName~BffRefreshEndToEndTests" -- RunConfiguration.TreatNoTestsAsError=true.
   • PublicAPI files are LF, sorted, and contain no stale IBffSessionStore lease signatures.
   • ADR 1305 is still free on a freshly fetched origin/main (other plans are in flight); renumber if not.
   • dotnet csharpier check . and ./build/dod.ps1 -Fast -Affected pass.

   Follow-ups

   • A failed lease release fails a finished BFF refresh: in DefaultBffUserTokenService.AcquireAndRefreshAsync the finally awaits ReleaseRefreshLeaseAsync, so a BffStoreUnavailableException there turns a refresh whose tokens were already written into a 503 and masks any exception from the refresh itself; catch and log the release failure instead, since the lease expires on its own.
3. polewskm

   closed this as completedin #460on Nov 2, 2026