fix(authz): route the bool surface through D5 + runnable §16–§19 example (F3) - #38
Merged
Merged
Conversation
D6's examples convention asks each SDK for one runnable example per new
feature. Seven of eleven had a D5 one; C# was among the four that did not.
examples/TelemetryHook aggregates in-process so it runs with no metrics
dependency, and demonstrates the D5 surface in a single run rather than
describing it. Pointed at nothing:
WARN: MaxRetryAttempts=25 was clamped to 3 (§16.1)
WARN: DecisionMemoTtl=00:01:00 was clamped to 00:00:05 (§17.1 rule 2)
check failed: checkAccess failed: HttpRequestException — Connection refused
--- telemetry ---
CheckAccess/Failure: count=3 mean=35ms
retries CheckAccess: 2
refreshes: 0
Both settings are deliberately out of range, and C# is the SDK where that has
the most to say: MaxRetryAttempts was publicly settable UPWARD before D5,
which is precisely what §16.1 forbids — a caller who can raise the cap turns
one client into the herd a backoff exists to prevent. D5 clamped it; this
example is where an operator finds out the clamp happened to them. It is the
only SDK whose run prints two clamp warnings, because it is the only one that
had two clamps to report.
The three failed attempts with two retries between them are the §16 budget,
and counting them at all depends on §19.2 rule 5 emitting one request pair per
ATTEMPT rather than per logical call.
Two decisions made for the reader rather than left to be rediscovered:
* RequestStartEvent is deliberately unhandled — RequestEndEvent carries the
same identity plus the outcome, so counting both double-counts.
* "Any AXIAM error" is spelled as `when (ex is NetworkError or AuthError or
AuthzError)`, because the §2 taxonomy is three sealed types with no shared
base in this SDK. Worth knowing before writing the same handler.
Verified: dotnet build -c Release, 0 warnings 0 errors; the output above is a
real run.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011ubrFbqsMkBqC5gwadPsDu
…y path CheckAccessAsync, CanAsync and BatchCheckAsync each posted directly to the transport: no §16 retry budget, no §17 memo, no §19 request pair. So the three most-used methods on AuthzRestClient were the three that did none of D5 — while D5ConformanceTests, which drove CheckAccessDecisionAsync, stayed green. That is the §16.7 failure mode seen from the other side. §16.7 was written because two SDKs shipped a retry surface that was exported, documented, tested and called by nothing; here the surface was called and the tests looked elsewhere. Either way the passing suite is what stops anyone from looking. The fix is delegation, not duplication: CheckAccessAsync returns CheckAccessDecisionAsync(...).Allowed, BatchCheckAsync maps BatchCheckDecisionsAsync, and BatchCheckDecisionsAsync now runs its attempt through RetryPolicy.ExecuteAsync with a telemetry span. One instrumented path and no second one to forget. Batch is retried (§16.2 names batch_check alongside check_access — the same side-effect-free POST, just plural) but deliberately not memoized: the §17 key is per-check, so a batch would fragment into n keys and a partial hit would need semantics §17 does not define. Six wire-count cases added, asserting the policy through the bool surface rather than the decision surface: retry, the attempt clamp, CanAsync inheriting the policy, the start/end/retry/start/end sequence, one memo shared across both surfaces, and BatchCheckAsync carrying /api/v1/authz/check/batch rather than a copy-pasted single-check template. Mutation-checked by forcing MaxAttempts to 1: four of the six fail, along with six pre-existing cases. Also corrects the class remarks, which claimed this class holds no local cache (the §17 memo is opt-in but real) and that the engine is "additive-only, allow-wins" — B1 shipped deny-override and closed SEC-040. 473 tests pass (45 AspNetCore + 428 Sdk). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011ubrFbqsMkBqC5gwadPsDu
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Two commits. The example came first; writing it is what exposed the bug in the second.
examples/TelemetryHook— the D5 example this SDK was missing.CheckAccessAsync,CanAsyncandBatchCheckAsyncdid none of D5.The bug the example found
CheckAccessDecisionAsynchad the §16 retry budget, the §17 memo and the §19 request pair.CheckAccessAsync,CanAsyncandBatchCheckAsynceach posted straight to the transport with none of them — so the three most-used methods onAuthzRestClientwere the three that skipped the whole of D5.D5ConformanceTestsstayed green throughout, because every §16 case droveCheckAccessDecisionAsync.That is the §16.7 failure mode seen from the other side. §16.7 was written because two SDKs shipped a retry surface that was exported, documented, tested and called by nothing. Here the surface was called and the tests looked elsewhere. Either way, the passing suite is what stops anyone from looking. The same defect was found and fixed in the PHP SDK (ilpanich/axiam-php-sdk#25); these two were the only SDKs with a bare-bool convenience method layered over a richer one.
The fix is delegation, not duplication
CheckAccessAsync→CheckAccessDecisionAsync(...).AllowedBatchCheckAsync→ mapsBatchCheckDecisionsAsyncBatchCheckDecisionsAsync→ newSendBatchAsyncrun throughRetryPolicy.ExecuteAsyncwith a telemetry spanOne instrumented path, and no second one to forget. Re-inlining a transport call into any bool method now fails the suite.
Batch is retried — §16.2 names
batch_checkalongsidecheck_access, the same side-effect-free POST, just plural — and is deliberately not memoized: the §17 key is per-check, so a batch would fragment into n keys, and a partial hit would need semantics §17 does not define. Conservative reading rather than invented ones.Also corrected
The class
<remarks>claimed this class holds no local cache (the §17 memo is opt-in but real) and that the engine is additive-only, allow-wins — B1 shipped deny-override and closed SEC-040.The example
Aggregates in-process, so it runs with no metrics dependency. Real output, pointed at an unreachable server:
This is the only SDK whose run prints two clamp warnings — because it is the only one that had two clamps to report.
MaxRetryAttemptswas publicly settable upward before D5, exactly what §16.1 forbids: a caller who can raise the cap turns one client into the herd a backoff exists to prevent. D5 clamped it; this example is where an operator finds out the clamp happened to them.Both settings are configured out of range on purpose, so
ConfigClampedEventis something you see rather than something the comments promise.Two decisions made for the reader:
RequestStartEventis deliberately unhandled.RequestEndEventcarries the same identity plus the outcome, so counting both double-counts. Someone copying this file gets that decided instead of discovering it from a metric reading 2× too high.when (ex is NetworkError or AuthError or AuthzError). The §2 taxonomy is three sealed exception types with no shared base here, so there is no single catch for it.Tests
Six wire-count cases through the bool surface, not the decision surface: retry, the attempt clamp,
CanAsyncinheriting the policy, thestart/end/retry/start/endsequence, one memo shared across both surfaces, andBatchCheckAsynccarrying/api/v1/authz/check/batchrather than a copy-pasted single-check template.dotnet test -c ReleaseMaxAttemptsforced to 1)dotnet build examples/TelemetryHook -c Releasedotnet runagainst an unreachable serverThe two new cases that survive the mutation are the memo-sharing one (no retry involved) and the attempt-cap one (which asserts against
RetryPolicy.MaxAttemptsitself); both fail against the pre-fix source, which is the mutation that matters for them.Contract §16.7's requirement — assert through the public surface, count requests on the wire — is what makes these tests able to catch this class of bug at all.
Generated by Claude Code