Skip to content

feat: phase 11 deviation reconciliation (docs/deviations.md, ledger gate, 26 repairs) - #106

Merged
Wahbeh-Mohammad merged 50 commits into
mainfrom
104-phase-11-deviation-audit
Oct 21, 2026
Merged

Wahbeh-Mohammad merged 50 commits into
mainfrom
104-phase-11-deviation-audit

Conversation

@Wahbeh-Mohammad

Copy link
Copy Markdown
Contributor

What and why

Phase 11 of the v1 roadmap: deviation reconciliation. Design and plan: docs/work/mvp/phase11/ (planning #105). Checklist: docs/work/mvp/phase11/2026-10-19-phase11-deviation-reconciliation-checklist.md. Ledger: docs/deviations.md.

Closes #104

Every design §10 deviation, §11 ambiguity resolution and §12 coverage cell was re-derived from source. Defects the audit surfaced are repaired TDD; a deviation is retained only with a stated reason class; documents are corrected where the code is right. The plan's six stacked PRs were built as commit groups on this one branch, each green at its gate task.

Repairs (PR 1–5 groups)

  • Gates: reproducible-pack.sh toolchain check + --self-test (NFR-12); core's dependency allowance is a fixed set, with a self-test (SEAM-1, NFR-1); FatalFilterArchitectureTests gates every broad catch and UrlRedactor no longer swallows OOM (RECOV-2); Disposal writes only to a recording activity (OBS-21); CA1062 on for src/, HttpResponseException rejects null / non-error status (XCUT-8).
  • Taxonomy and bridges: Breaking ServiceRequestException : IOException (TRANSPORT-20, XCUT-4); Breaking ServiceResponseException removed; AsAsync faults instead of throwing (ASYNC-2); new ResponseTaskExtensions.WaitOrDisposeAsync (SEAM-30); validating init accessors name their field (SEAM-29).
  • Bodies, headers, pages: Breaking a second open returns the same stream (BODY-14, HTTP-41); inbound header names are not trimmed (HTTP-19, XCUT-18); FromFile rejects a Unix FIFO/device (BODY-11, HTTP-40); Request rejects a non-canonicalizing Uri; Page snapshots its values (PAGE-2); the query splice keeps empty segments (PAGE-21).
  • Transport: SDK-built handlers stop SocketsHttpHandler's native re-send via FirstResponseByteStream (TRANSPORT-2); the transport-2 waiver now applies only to a caller's handler.
  • Observability, config, retry, SSE, auth: TraceIdGeneration (W3C / Datadog, OBS-27); null log fields are "null" (OBS-3); [LoggerMessage] generator for plain-name events; an empty config value falls through to the lower provider (CFG-2, registered root); RetryOptions.PacingHeaders (RETRY-21); SSE per-event data cap + ServerSentEventTooLargeException (SSE-19); the attempt header replaces its value and never overwrites the idempotency key (RETRY-38); an unexpected MD5 probe failure declines MD5 (AUTH-15).
  • Flakes: RecordingTaskSchedulerTests, DisabledPathAllocationTests, UntracedAllocationTests stabilised (the named OperationSpanLifecycleTests flake did not reproduce in >70 runs).

Ledger and close-out (PR 6 group)

  • docs/deviations.md (33 §10 rows, 72 readings, negative scans N1–N20) guarded by LedgerAuditTests (L1–L11) in CI.
  • §12 generated by requirement-coverage.cs --section12, checked by --check-section12 (new CI step).
  • Thread.Abort and ModuleInitializerAttribute banned (RS0030).
  • Design §2–§12 dated corrections, docs/first-release.md walk, styleguide overlay, CLAUDE.md, user pages, knowledge notes, roadmap status note.

Requirements

Unmet-clause count 4 of 5 (planned 3): AUTH-15 (FIPS host), PAGE-21 (unreserved residual), TRANSPORT-14 (name half), and CFG-2 for a section of a configuration not registered in the container (no AOT-safe public route to a section's root; found in review, counted under P11-2). §10 entries 6, 7, 24, 28, 31, 33 retired. MUST ⏳: 1 (NFR-17). Checklist rows cite every repaired ID.

Verification

Linux, .NET SDK 10.0.401, on the final tip.

  • dotnet build (Release, 0 warnings), dotnet format --verify-no-changes, dotnet test --solution Dexpace.Sdk.sln: 7 687 tests, 0 failed, 23 skipped
  • Coverage 97.5% (floor 80%) and its self-test; dotnet pack; dependency audit + new self-test; packaging conformance; reproducible pack + new self-test
  • NativeAOT smoke publishes with no warnings and prints all checks passed (CheckPhase11Async, CheckPhase11ObservabilityAsync)
  • Failing-first tests for every repair, each with its [Trait]; three new Security tests, none removed or loosened
  • PublicAPI.Unshipped.txt updated
  • requirement-coverage.cs --check / --check-section12, housekeeping probe (no drift found), verify-structure, tools solution

Records

  • CHANGELOG.md [Unreleased] Phase 11 section, breaking changes marked
  • Deviations recorded in docs/deviations.md and design §10 (dated corrections)
  • docs/first-release.md updated (one-time public-surface list, unsatisfied MUSTs, asymmetries)
  • Checklist, package READMEs and docs/sdk-documentation/ pages updated
  • Frozen-tree edits are dated, stated corrections only

Open items for the lead

  1. reproducible packages CI job — fixed on hypothesis (a) (runner SDK patch ≠ global.json) without reading feat: phase 10 cross-cutting invariants and conformance (XCUT-1..-24, NFR-1..-17, appendix B) #102's failing log; this PR's CI run confirms or the script's diagnostics name the differing entry.
  2. NFR-17 — add the required_status_checks rule to the main ruleset (text in the phase 10 checklist).
  3. D3 (webhooks post-v1) unchanged.
  4. TarWriter special-file classification verified on Linux only; the macOS row is its first run elsewhere.
  5. ~15 init accessors still report ParamName value for range/format rejections.

Replaces HttpLogRecordTests.A_null_value_is_carried_as_null, which asserted the
old behaviour (a null structured value stays null), with
A_null_value_is_emitted_as_the_literal_null.
…-14, HTTP-41)

BODY-14 asks that requesting a response body's read handle repeatedly returns the
same underlying handle, not a replay and not a failure. The stream-backed and
in-memory bodies of ResponseBody and the SystemNet transport body now cache the
stream the first open obtained and return it to every later open, in either form.
A body disposed after it was opened reports StreamClosedException on a later open
(it reported StreamConsumedException first); one disposed before any open is
unchanged. The convenience readers after a manual open read the remaining bytes of
the same handle.

HttpResponseException.GetError/GetErrorAsync map the ObjectDisposedException a
second read now meets (the first read disposed the shared stream) to the same
ResponseNotReadException they threw for StreamConsumedException.

The response-logging wrapper is not changed: past its capture cap its tail stays
single-use and a second open throws StreamConsumedException, because BODY-24 (MUST)
says a second read in that regime fails. The plan listed LoggingResponseBody; the
more specific rule wins and the test now also pins the message.

New tests: ResponseBodyLifecycleTests.A_second_open_returns_the_same_stream,
An_open_after_dispose_still_throws_StreamClosedException (the unopened-stream row is
a pin), A_typed_reader_after_a_manual_open_reads_the_remaining_bytes_of_the_same_handle;
HttpResponseMessageBodyTests.A_second_open_returns_the_same_stream.

Assertions re-stated against BODY-14's letter (none is a Security test):
- ResponseBodyTests.OpenRead_after_OpenReadAsync / OpenReadAsync_after_OpenRead now
  Assert.Same instead of StreamConsumedException
- ResponseBodyLifecycleTests: read-then-dispose-then-open reports closed, not consumed;
  A_second_open_throws_and_names_the_buffering_route is folded into the BODY-24 test
- LoggingResponseBodyTests.Over_cap_a_second_read_throws_StreamConsumedException keeps
  its throw and gains the message assertions
- ReadValueTests, BodyConvenienceTests, ReadValueEndToEndTests: a second typed read
  after the first disposed the body is StreamClosedException
- BodiesAndRequestTests.ResponseBody_SecondReadThrows: StreamClosedException
- HttpResponseExceptionGetErrorTests.GetError_throws_ResponseNotReadException_on_a_consumed_body:
  inner exception is ObjectDisposedException
- HttpResponseMessageBodyTests: shared handle in either order; read-dispose-read and
  OpenRead-after-dispose report closed
The script pins each checkout's toolchain with a job-local global.json
(rollForward disable), prints dotnet --version from inside it and exits 1
naming both versions when it is not the pinned one. --self-test packs once,
requires an identical copy to compare clean, flips one byte in one .nupkg of
the copy and requires the compare to exit 1 naming it, and requires the
toolchain check to refuse another version. The job runs it and CiWorkflowTests
holds the step.
…ce (HTTP-19, XCUT-18)

Headers.Builder.AddInbound shared the outbound name rule, which trims SP and HTAB at
either end before validating, so a received " X-A" or "X-A\t" was stored as "X-A".
HTTP-17's trim is outbound only (design section 11 item 70): HTTP-19 keeps received
names strictly validated and XCUT-18 refuses a name with HTAB. HeaderSyntax gains
ValidateInboundName (the token grammar, no trim) and AddInbound uses it; the outbound
trim tests stay green unedited. The reference transport never reaches it (the runtime
refuses such names), so this closes the path for third-party transports and direct
callers.

Security test added (never edited afterwards):
HeaderInjectionValidationTests.The_inbound_path_does_not_trim_a_name.

Kit: xcut-18.model-validation now requires an inbound name with HTAB at one end to be
refused (the outbound check still accepts a refusal or a trim, HTTP-17). A new negative
control, a model that trims an inbound name, is registered with the other xcut-18 rows;
the existing inbound control's expected reason follows the new message.
…(CFG-2)

Replaces EmptyValueTests.An_empty_value_over_a_lower_providers_value_resolves_the_record_default
(the old shadowing assertion) with An_empty_value_falls_through_to_the_lower_provider and
keeps the merged read as a pin for a section from an unregistered configuration.
The audit allowed core the logging facade's whole closure, so a dependency the
facade grows later would have passed. It now allows exactly the facade and
Microsoft.Extensions.DependencyInjection.Abstractions (SEAM-1, NFR-1; design
section 10 entry 1). dependency-audit-selftest.sh injects a package under the
facade, under its dependency and with no edge into copies of core's deps.json
and requires exit 1 naming it. The step and its CiWorkflowTests row join.
… HTTP-40)

The shared framework reports a FIFO or a character device as a file of length 0, and
opening a FIFO for read blocks until a writer appears, so RequestBody.FromFile let a
special file through as an empty upload (design section 10 entry 31 called this
unfixable). System.Formats.Tar is in the shared framework and classifies a path by its
file type to build an archive header without opening a special file, so a size-0 path
on a non-Windows host is now written as one entry into an in-memory archive and the
entry's type read back (UnixFileKind.SpecialKindOf). A regular file passes; a FIFO, a
character or block device, a socket or anything else throws ArgumentException naming
the kind, never the path. A symbolic link is classified as its target. Windows keeps
its device-attribute check.

Tests (FileRequestBodyTests, Unit): a character device, a FIFO (made with mkfifo,
constructed on a worker under a 5 s guard to prove it does not block), a link to
/dev/null, a Unix domain socket; An_empty_regular_file_is_accepted is a pin, proven
able to fail by breaking the regular-file arm. The NativeAOT smoke gains
CheckPhase11Async (tests/Dexpace.Sdk.AotSmoke/SmokeChecks.Phase11.cs) with the
/dev/null rejection on Linux and the same-stream second open; the publish prints no
warning and the dependency audit stays green.
…broad catch

The two redaction backstops caught Exception unfiltered, so a fatal exception
was turned into the sentinel (RECOV-2, RETRY-25, XCUT-20; design section 10
entry 12). CA1031 is silent on a filter that is always true and on a rethrow,
so FatalFilterArchitectureTests scans the four runtime libraries: an
unfiltered catch-all must end in throw; (or rethrow a fatal exception) and a
filtered one must exclude fatal exceptions in its when. A theory over inline
snippets proves the scanner flags a when (true) and a swallowing bare catch.
A Uri created with UriCreationOptions.DangerousDisablePathAndQueryCanonicalization keeps
its path and query exactly as written. HttpClient sends such a Uri verbatim, a raw CR or
LF in the query included, straight into the request line (design section 10 entry 18,
V9); Request accepted it, and the wire re-check covers headers, not the target. The
runtime marks such a Uri by refusing to give back its path or query components
(GetComponents(Path | Query) raises InvalidOperationException), which a Uri the default
parser made never does. Request's one validating constructor now calls a private
RequireCanonical(Uri) that maps that to an ArgumentException (ParamName url) naming the
cause and not the input; so do WithUrl and every other With* derivation.

Security test added (never edited afterwards):
HeaderInjectionValidationTests.A_uri_created_without_canonicalization_is_rejected
(constructor and WithUrl, three Uris including one with a raw CR/LF). Pin:
RequestTests.A_url_the_default_parser_made_is_accepted over the shapes the default
parser escapes or normalises, proven able to fail by making the check throw.
Page<T> stored the list it was given, so a Pageable.FromFetchers page whose fetcher
went on to change the list changed after the engine had yielded it (design section 10
entry 17: the page is a value readable forever). The constructor now copies into its
own read-only list; PageStep drops its now-duplicate copy and passes the strategy's
items straight in.

Tests (PageTests, Unit): Values_is_a_snapshot_of_the_list_passed_in,
Values_cannot_be_changed_through_a_cast, An_empty_list_gives_an_empty_page.
Page_Constructor_SetsProperties asserted Assert.Same(values, page.Values), the old
identity, and now asserts equal and not the same.
QuerySplice split the raw query with RemoveEmptyEntries, so a next-page URL with a stray
'&' (?a=1&&b=2&) lost it when a cursor or page number was set, although PAGE-21 says an
untargeted parameter is copied byte for byte (design section 10 entry 18). An empty
segment is an untargeted parameter: Segments now keeps it, a query with no characters has
no segments, and an empty segment is never matched because a parameter name is never
empty.

QuerySpliceTests.Stray_empty_segments_are_skipped_matching_http_31_query_parsing pinned the
dropped form and is replaced by Stray_empty_segments_are_copied_verbatim
(?a=1&&b=2& with cursor=abc yields a=1&&b=2&&cursor=abc); An_empty_query_has_no_segments_so_a_set_appends_without_a_stray_separator
pins the empty-query edge. The unreserved-character residual (entry 18, counted) remains.
…listeners

The class asserts the zero-allocation disabled path (OBS-1) but ran in the
Instrumentation collection, parallel to classes that install a process-wide
ActivityListener. A listener appearing between the two phases of
The_redacted_url_is_not_computed_on_the_disabled_path made the instrumented
pipeline start an Activity per call, failing it as 'short 745 B, long 995 B'
in 4 of 60 runs under load. It joins NoDiagnosticListeners, the collection
whose documented purpose is exactly the zero-allocation assertions; 60 runs
of the fixed build were green.
It_runs_each_task_on_its_own_non_pool_thread asserted that two sequential
tasks ran on threads with different Environment.CurrentManagedThreadId, but a
managed thread id is recycled once its thread has exited, so the second
thread could reuse the first's (Expected: Not 52, Actual: 52, 3 of 60 runs
under load). It now compares the Thread objects.
…(OBS-21)

An Activity whose sampler only propagates context (IsAllDataRequested false)
still records AddException, so the quiet dispose wrote an exception event to a
span that must be inert (design section 10 entry 23, V14). The write is
guarded by IsAllDataRequested. DisposalTests joins NoDiagnosticListeners
because the new test installs a non-recording listener.
…2 on for src (XCUT-8)

The public constructor dereferenced a null response (NullReferenceException)
and built 'The server returned an error response: OK(200).' for a successful
one. It now throws ArgumentNullException / ArgumentException (ParamName
response) from a guard inside the base call. CA1062 is an error for src/: it
had exactly one finding, this constructor (V12), which retires design section
10 entry 28.

Callers that relied on the unguarded constructor, edited here: the two
conformance controls that built a successful exception now use a bare
SdkException (they stay red for xcut-8), and
RetryClassifierTests.Every_status_100_to_599_matches_the_classifier builds the
exception for error statuses only and asserts the refusal below 400 (the plan
named the controls; this test was not on its list). None is a Security test.
Breaking: CHANGELOG.
Edits ServerSentEventReaderTests.The_reader_holds_exactly_the_reviewed_instance_fields
(the reviewed field list gains _maxEventBytes) and renames
A_hostile_stream_of_many_capped_lines_with_no_blank_line_is_the_documented_residual_R1
(residual R1 is closed, the test keeps its 100-line event under the default cap).
The Security class ServerSentEventLineCapTests gains a test and rewrites its
summary sentence that left R1 unpinned; no assertion is removed.
…construction

Replaces Md5AvailabilityTests.Probe_does_not_swallow_other_exceptions, which
asserted that an InvalidOperationException escapes the probe, with
An_unexpected_probe_failure_means_refused and A_fatal_probe_failure_still_propagates.
TRANSPORT-20 requires a transport failure to be a subtype of the platform
IOException and XCUT-4 that it be catchable as a generic I/O error; the port
had rooted it in SdkException because it chose one SDK root, which no
requirement asks for (design section 10 entry 7, P11-7). ServiceRequestException
(and ServiceRequestTimeoutException) now derive from IOException and implement
IRetryableError directly; the suppressed trail is stored through
ExceptionTrail and rendered by ToString through one renderer shared with
SdkException (SuppressedTrailRenderer), so a cycle across the two families
still terminates. The retry classifier reads IRetryableError and the cause
chain, so retry behaviour is unchanged.

Kit assertions that asserted the old base now require IOException and
IRetryableError (FailureAssertions, TaxonomyInvariants, RedirectAndTimeout,
Cancellation, Concurrency, Resend). The negative forms
(failure is not SdkException { IsRetryable: true }) would have gone vacuous and
now read IRetryableError; the plan listed only five of the eight sites. The
xcut-4 control that turned a refusal into a bare SdkException now checks the
IOException clause, and a new control throws a non-retryable IOException for the
retryable clause. Tests that read .Suppressed on a transport failure use
ExceptionTrail.GetSuppressed; RetryRecoveryTests asserts IRetryableError instead
of SdkException for the exhausted failure.

Breaking: catch (SdkException) no longer catches a transport failure; catch
ServiceRequestException or IOException (CHANGELOG).
…NSPORT-2)

SocketsHttpHandler re-sends a request with no content whose connection ends before any
response byte, up to three more times, and has no switch to stop it (design section 10
entry 33). A body-less POST/PUT/PATCH is given a zero-length content so the retry cannot
fire; GET, HEAD, OPTIONS, DELETE and TRACE cannot be given content (TRANSPORT-26, RFC 9110
section 8.6) and arrived four times. The runtime takes the retry on a zero-byte first read,
not on an exception, so SystemNetHttpClient.CreatePrimaryHandler (used by the owned
construction and by the DI named client) now installs a PlaintextStreamFilter: a
pass-through FirstResponseByteStream that tracks "a request was written and no response
byte has been read" and, in that state, turns a read of 0 into the
HttpIOException(ResponseEnded) the runtime raises itself, so the failure keeps the error
code it had and the request reaches the server once. No byte is inspected or altered; a
zero-count read, an idle close and the end of a response that began with a byte pass
through; HTTP/2 connections are not wrapped. A handler or HttpClient the caller supplies
is the caller's native client and keeps its retry (design section 11 item 71).

Tests: BodylessRetryWireTests.A_bodyless_request_reaches_the_server_once_on_an_sdk_built_handler
(five methods x async/sync x owned / CreatePrimaryHandler-based; red: four arrivals) replaces
the residual theory, A_caller_handler_keeps_its_native_retry pins the scope, and four
success-path rows prove the filter changes nothing a working exchange sees (a response
delimited by the end of the connection, one keep-alive connection across methods and a 200 KB
upload, 24 concurrent callers over the pool); FirstResponseByteStreamTests pins the rule on a
scripted stream.

Task 4.2 is folded into this commit, because convention 10 requires the waiver to go in the
change that fixes its clause (a stale waiver fails the run): the transport-2.bodyless-not-resent
waiver is deleted for the owned SystemNet subject and the DI named client and kept, with its
reason now "a caller's handler is the caller's native client (design section 11 item 71)",
for the handler-taking subject only, which is given a SocketsHttpHandler the caller built.
SystemNetSubject.OptionsFor(kind) replaces the shared Options; the stale rule was seen to fail
the owned subject with the waiver put back. The TRANSPORT-14 waiver text drops "no
handler-level workaround exists" (a stream filter can drop the line; the transport still
declines to rewrite the head, design section 10 entry 32). The AOT smoke's conformance slice
gains transport-2.bodyless-not-resent.
…d response truthfully

ServiceResponseException was public, documented as XCUT-4's third transport
shape, and thrown by nothing (found by phase 10's measurement). With
ServiceRequestException now the one transport branch, the type is deleted with
its five public API lines. SendFailures chooses the message by HttpRequestError:
'could not be sent' only for name resolution, connect, TLS and proxy-tunnel
failures, 'no response was received' for the rest, so a response that ended
after the request arrived is no longer called unsent.

Tests that constructed the type use ServiceRequestException: RetryClassifierTests
(three), RetryPolicyTests, RetryPolicyEngineTests, OperationEventTests, the
ResponseShapeControlTests control and the raw-socket client, whose unparseable
status line is now a ServiceRequestException with its own message.
TransportExceptionTaxonomyTests.Core_exports_one_transport_failure_family pins
the family by reflection. Breaking: CHANGELOG.
A TaskScheduler whose QueueTask throws made AsAsync(scheduler).ExecuteAsync
throw TaskSchedulerException synchronously from the call, and so did a
pre-cancelled token (V4, F-A2): a failure detectable while constructing the
async operation must come through the future's failure channel. A signalled
token now returns Task.FromCanceled before the scheduler is asked, and the
StartNew is wrapped so a rejection becomes a faulted task.
RejectingTaskScheduler (TestSupport) counts the QueueTask attempts it refuses.
Breaking (behaviour): CHANGELOG.
…stops waiting (SEAM-30)

Design section 11 item 43 documented Task.WaitAsync(token) on the bridge's task
as the cancel-without-interruption mode, but the V5 probe leaked an undisposed
response in 44 of 20 000 runs: the call completes at the instant the token is
signalled, the wait observes the token first and nobody closes the response.
LateResult already closed it but was internal and had no caller. The new public
ResponseTaskExtensions.WaitOrDisposeAsync(this Task<Response>, CancellationToken)
delegates to it; AsAsync's docs name it as the way to cancel without
interruption, and LateResult's remark now names both WaitAsync sites.

WaitOrDisposeTests runs the V5 race 20 000 times through the bridge (8 workers,
jittered spin on both sides) and requires every caller to get a live response or
an OperationCanceledException and every produced response to be closed exactly
once. The harness was proven able to fail: with a raw WaitAsync in its place it
left 5 responses undisposed in each of 3 runs.
An init accessor's parameter is called value whatever the property is, so
ArgumentNullException.ThrowIfNull(value) reported ParamName == "value", which
names nothing (design section 10 entry 10). All 37 null-rejecting accessors in
src now pass nameof(Property): AuthRequirement, DexpaceClientOptions,
ProxyOptions, RedirectOptions, RetryOptions, RequestOptions,
OperationDescriptor, IdempotencyKeyStep, SystemNetHttpClientOptions and the
conformance kit's subjects and options, plus the three spellings the plan's
grep did not match: FetchedPage<T>.Page (a ?? throw), and the null-element
checks of RedirectOptions.AllowedMethods and ProxyOptions.NonProxyHosts.

InitAccessorParamNameTests holds one row per accessor (core and kit; the
transport's two are in its own project) and re-scans the sources through
TestSupport's InitAccessorScanner, so a new accessor needs a row and a bare
ThrowIfNull(value) fails. Two existing tests that asserted ParamName "value"
(OptionsImmutabilityTests, OperationDescriptorTests) now assert the property
names. Breaking (behaviour): CHANGELOG.
L1 to L11 as permanent tests over the real files plus a fixture that must fail each
(written first against a skeleton that finds nothing); BannedSymbols.txt bans
Thread.Abort and ModuleInitializerAttribute; requirement-coverage.cs gains
--section12, --check-section12 and the 'supersedes' reading of a carried row, with
self-test cases; docs/first-release.md's Unsatisfied MUSTs is the three clauses the
ledger counts, which rule L6 reads. The Section 12 CI step lands with the table (6.4).
Owning rows for the IDs a repair changed (each says it supersedes the closed row it replaces),
the 15 MUST hand-offs phase 10 listed, five SHOULD/MAY hand-offs and ASYNC-4's vacuous mark;
requirement-coverage.cs gains the rule that a deferred MUST names its owner, with self-test
cases. MUST rows still hourglass: 16 before, 1 (NFR-17) after.
Design §10 and §11 gain one dated correction each (the count of three, the retired entries, every
verdict, items 69 to 72), §12 gains its generated table between markers, and every section of §2-§9
gains a dated As built line; the ledger's reading of item 18 follows. The blocking CI step Section 12
(requirement-coverage.cs --check-section12) and its CiWorkflowTests row join with the table they hold.
The_untraced_path_does_not_compute_the_redacted_url took one allocation
reading per URL, where the other tests of the class take the best of six
rounds. A single round failed 3 of 21 full-suite runs (short 665 B, long
890 B per call against a 200 B bound) and none of 30 after the change. A
redaction that ran would still add tens of KB to every round of the long URL.
…eader (RETRY-38)

RetryOptions.AttemptHeaderName refused only the literal Idempotency-Key, but IdempotencyKeyStep.HeaderName is
configurable, so the same custom name on both made the attempt stamp replace the key with the attempt number on every
send of a retried POST. The pair is now refused where each stack is assembled: PipelineBuilder.Build, a per-call
DexpaceClientOptions override on HttpPipeline (before anything is sent), and the RecoveryDispatcher constructor.
… the socket failure to socket

The Unix type check wrote the path into a tar archive, and the tar writer opens a regular file to attach its content, so
an empty regular file with mode 000 failed construction with an UnauthorizedAccessException carrying the full path, and any
plain IOException (EIO, ESTALE, a lock) was reported as a socket. A refusal to open now classifies the path as regular
(the writer opens no special file), and only the managed IOException the writer throws for a file type it has no tar type
for is a socket.
…adException (BODY-14)

The body returns the first stream on a second open, and the content stream SocketsHttpHandler hands out reads 0 bytes
after it is disposed instead of throwing, so a second HttpResponseException.GetError(Async) decoded an empty payload.
The transport body now hands out a stream that reports its own disposal (ObjectDisposedException on any use), which
GetError already reads as already consumed, as it does for every other body.
Two concurrent first opens both called HttpContent.ReadAsStream(Async), and a response content can be consumed once, so the
second caller got an InvalidOperationException from SocketsHttpHandler's content (or an HttpRequestException for a sync
read of an async-obtained stream). The first open now runs under a gate taken by both forms; a queued open takes the
caller's token, a failed opener leaves the next open to try, and a body disposed mid-open reports StreamClosedException.
Corrects the comment and the checklist line that described a latch the body no longer has.
…ns that accepted a raw IOException

transport-18's body-source clause and transport-7.attempt-timeout-aborts were loosened to "an IOException" when transport
failures moved under IOException, so a third-party transport that let the body source's own exception or a raw IOException
escape unmapped passed. Both now require an IOException that is an IRetryableError, each with a negative control.
…ays L3 checks

Rules L9 and L10 skipped a path glob that did not parse or whose directory held no file, so a renamed directory made a
negative scan pass vacuously. Each is now an L10 finding with a fixture that must fail. L3 stays an existence check and
CLAUDE.md now says so.
…eamConsumedException

CLAUDE.md (three places) and bodies.md said ResponseBody.FromStream throws StreamConsumedException on a second read. It never
does: the first convenience read disposes the body, so the second reports StreamClosed, and a convenience read after a manual
open and drain returns the empty remainder of the same handle. StreamConsumedException is the request side's second write and
the logging wrapper's tail past its cap. Pinned by two tests.
…not close (TRANSPORT-2)

The filter is armed by a request's first write, so a read of 0 before it passes through (an idle close is not a failed
request). A server close landing after the runtime's reuse check of a pooled connection and before that write can still make
the runtime re-send once, a request whose first copy went to an already-closed connection. Reasoned from the runtime's
source, not reproduced, and not closed because a verdict on the idle read would change what the pool sees. 'Reaches the
server once' now says not up to four times, not never twice.
… unmet clause

The empty-value fall-through walks the providers of the root a section was taken from, and no public API reaches that root
from an IConfigurationSection, so a section of any configuration other than the container's registered root keeps the merged
read and an empty value still hides the lower provider. By the P11-2 counting rule the outcome is not delivered on that domain,
so it is a counted clause: the checklist row splits, entry 25 becomes a retain with unmet-clause CFG-2, the count line, the
register's Unsatisfied MUSTs and the generated section 12 table agree at four of five, and di-empty-shadows is restored,
narrowed. CLAUDE.md and the DI page and README now qualify the fall-through.
…untime's source

The previous commit's wording claimed the window was reasoned from the runtime's source; it was reasoned from how a pooled
connection is reused, and no test can land a close inside it.
@Wahbeh-Mohammad

Copy link
Copy Markdown
Contributor Author

Review summary

An independent adversarial review of the full branch (main...aa88e25) was run before opening this PR. Each finding was then verified against the code (probe or reading), and the verified findings were fixed on this branch with failing-first tests (aa88e25..c352b6d).

# Severity Finding Verified Resolution
1 medium RETRY-38: AttemptHeaderName refused only the literal Idempotency-Key; with a custom IdempotencyKeyStep.HeaderName equal to the attempt header, every retry overwrote the key confirmed (probe) Fixed 5126fcc: the collision is refused at PipelineBuilder.Build, on a per-call options override, and in RecoveryDispatcher (case-insensitive); 15 tests, both stacks, sync + async
2 low FromFile's empty-file type check (TarWriter) opened the file: a mode-000 empty file failed with a path-bearing UnauthorizedAccessException; any plain IOException read as "socket" confirmed (probe) Fixed 117c264: unreadable empty regular files accepted again, "socket" only for the writer's own refusal, no path in messages; docs/CHANGELOG corrected
3 low A second GetErrorAsync over the SystemNet body deserialized an empty payload (a disposed content stream reads 0) instead of throwing ResponseNotReadException confirmed (probe) Fixed 0dc5955: DisposalAwareStream throws ObjectDisposedException after dispose, matching core; wire tests sync + async
4 low CFG-2 fall-through only applies when the section belongs to the container-registered IConfiguration; the residual was unrecorded confirmed (reading) Documented and counted 4a86bb4: no AOT-safe public route to a section's root; checklist row split, §10 entry 25 now unmet-clause: CFG-2, count 3 → 4 of 5, di-empty-shadows restored narrowed
5 low Two kit assertions (transport-18 body source, transport-7 attempt timeout) accepted a raw, unmapped IOException confirmed (reading) Fixed d38a1d9: require IOException + IRetryableError; two new negative controls
6 low Ledger gate: a glob resolving to no files made a negative scan pass vacuously; CLAUDE.md overstated L3 confirmed (reading) Fixed 8964fea: unparseable / empty globs are findings (3 fixtures); CLAUDE.md states what L3 checks
7 low CLAUDE.md named StreamConsumedException for a second read of ResponseBody.FromStream (actual: StreamClosedException) confirmed (probe) Corrected ca1b8ef, with two pin tests
8 low FirstResponseByteStream: a micro-window on an idle pooled connection can still let the native retry fire plausible (reasoned, not reproducible) Documented f970144, c352b6d: remarks, transport.md, checklist/ledger rows, new idle-connection-native-retry asymmetry; overclaims softened
9 low Concurrent first opens of a SystemNet body could throw InvalidOperationException; the comment described a latch that no longer existed plausible → confirmed by the new real-response test Fixed cb6a68e: first open serialized across both faces; 13 deterministic cases + a real-response concurrency test (red before)

The reviewer also checked and found sound: the transport filter against stale keep-alive reuse, connection-delimited and HTTP/1.0 responses (no false throw; HTTP/2 untouched), the same-stream body change, the IOException re-rooting (no src/ catch site misses it), AsAsync / WaitOrDisposeAsync, the SSE cap (sticky, sync/async parity), the provider walk, the Request canonicalization check, CA1062, and that no Security test was loosened or removed.

Post-fix gate (Linux, SDK 10.0.401): build 0 warnings, format clean, 7 687 tests, 0 failed, 23 skipped; Conformance and Security categories green; --check / --check-section12 green; AOT smoke passes; housekeeping probe clean.

@Wahbeh-Mohammad
Wahbeh-Mohammad merged commit 6b49af6 into main Oct 21, 2026
4 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Phase 11: deviation reconciliation

1 participant