Skip to content

feat: phase 9 host integration (CFG-1..-38 subset, SEAM-5, SEAM-6, OBS-35) - #98

Merged
Wahbeh-Mohammad merged 1 commit into
mainfrom
96-phase-9-host-integration
Oct 13, 2026
Merged

Wahbeh-Mohammad merged 1 commit into
mainfrom
96-phase-9-host-integration

Conversation

@Wahbeh-Mohammad

Copy link
Copy Markdown
Contributor

What and why

Phase 9 of the v1 roadmap: host integration, the new package Dexpace.Sdk.Extensions.DependencyInjection. Design and plan: docs/work/mvp/phase9/ (planning #94, #95). Checklist: docs/work/mvp/phase9/2026-10-12-phase9-host-integration-checklist.md. User page: docs/sdk-documentation/dependency-injection.md.

Closes #96

  • Registration: AddDexpaceClient(name[, IConfigurationSection]) returns a sealed DexpaceClientBuilder (Configure, ConfigureTransport, ConfigureCredentials, UseTransport, UseSerde, UseAuth, UseAuthPolicy, AddPolicy, UseProxyFromEnvironment, HttpClient). The client is registered as keyed singletons (HttpPipeline, IAsyncHttpClient, ISerde, DexpaceClientOptions).
  • Configuration: a hand-written reader, not the configuration binder (the source generator binds no init accessor; the reflection mode is not AOT-safe). Base-10 integers, strict booleans, CFG-7 durations, enums by name, lists that replace defaults. An empty value is absent. Every bad key, rejected value and unknown key (at any depth, group keys included) fails ValidateOnStart as "<path>: <rule>", never quoting a configured value. ConfigurationBinder is banned in src/.
  • Named client: built over IHttpClientFactory with a redirect-off primary (SystemNetHttpClient.CreatePrimaryHandler, the one place the owned-handler invariants are built), no factory loggers, an infinite handler lifetime with a pooled-connection lifetime, and a container-owned lease that releases the primary.
  • Single registration: one transport and one codec per client; an explicit install replaces the default; a different second install throws at the call; keyed duplicates fail at start.
  • Gates: scripts/ci/dependency-audit.cs holds each library to a policy record (one third-party root plus the first-party projects it names). A third conformance driver runs the kit over the DI-built transport on both faces with the two permanent waivers. New Security class NamedClientRedirectWireTests (phase 1 S3 row 9). AOT smoke covers the package.

Requirements

15 owned rows: CFG-1, -2, -3, -4, -5, -6, -7, -10, -11, -14, -37, -38, SEAM-5, SEAM-6, OBS-35. 13 ✅ / 2 N/A / 0 🚫 / 0 ⏳.

  • Split ✅: CFG-2 (fall-through to a lower tier, §10 entry 25), CFG-5, CFG-6, CFG-7, OBS-35 (never-throw clauses, §10 entry 26).
  • N/A: CFG-4, CFG-10 (vacuous in this port).
  • SEAM-5, SEAM-6: ✅ for the DI half; the DI-less half stays N/A (2b).
  • Evidence added to rows owned elsewhere: SEAM-7, TRANSPORT-1, XCUT-10.

Verification

Linux, .NET SDK 10.0.401.

  • dotnet build (Release, 0 warnings), dotnet format --verify-no-changes and dotnet test --solution Dexpace.Sdk.sln green: 6 857 tests, 0 failed, 22 skipped
  • Coverage 97.61% (floor 80%) and its self-test; dotnet pack; dependency audit (5 libraries conform); reproducible pack byte-identical
  • NativeAOT smoke publishes with no trim/AOT warnings and prints all checks passed
  • Red evidence: see the checklist's Red evidence paragraph and the review record below
  • PublicAPI.Unshipped.txt updated (new package; SystemNet gains CreatePrimaryHandler), new packages.lock.json files committed, locked restore clean on both solutions
  • Housekeeping probe clean; tools solution builds and its tests pass; scripts/knowledge verify-structure OK

Records

  • CHANGELOG.md [Unreleased] entries (Added, plus the XCUT-10 resilience-handler guidance)
  • Deviations recorded: §10 entries 25 and 26 extended; §11 items 67 and 68 added
  • docs/first-release.md updated
  • Checklist, package READMEs and docs/sdk-documentation/ pages describe the code
  • Design-doc edits are dated, stated corrections only

Lead rulings

Taken at the recommended defaults: P9-1, -2, -4, -5, -7, -9, -10, -12, -15, -16, -17, -19, -20, -28, -29. Worth reviewing:

  • P9-7 (CFG-2): an empty value silently shadows a lower provider's value (option B would fail start).
  • P9-28: a numeric accessor's failure message may echo the number.
  • P9-29: the duration/list grammar readings (§11 items 67, 68).

…S-35)

Adds Dexpace.Sdk.Extensions.DependencyInjection: AddDexpaceClient and the
sealed DexpaceClientBuilder register a client as keyed singletons over a
named IHttpClientFactory client with a redirect-off primary handler and a
container-owned lease that releases it. Settings are read by a hand-written
configuration reader (the binder cannot set init accessors and is not
AOT-safe) and validated on start without quoting configured values.

Also: SystemNetHttpClient.CreatePrimaryHandler, the ConfigurationBinder ban,
a per-library policy record in the dependency audit, a third conformance
driver over the DI-built transport, NamedClientRedirectWireTests (Security),
AOT smoke coverage, the checklist, the user page and dated design corrections.

Closes #96
@Wahbeh-Mohammad

Copy link
Copy Markdown
Contributor Author

Review record

Built in one pass from the design and plan, reviewed read-only against the design and spec with every finding reproduced before it was reported, then the confirmed findings were fixed.

Confirmed findings (all fixed)

# Severity Where Finding Fix
1 medium ConfigurationReader.cs (Walk, group branch) A value set on a group key was silently ignored, against P9-9 / plan R9 ("unknown keys fail at every depth"). Transport:Proxy = http://user:pw@proxy:8080, Auth:ApiKey = …, Auth = Bearer … and Retry = 5 all validated clean: the proxy and the credentials were dropped, so the failure surfaced as direct traffic or 401s at run time. Reproduced with a probe over IStartupValidator.Validate(). A non-empty group value now fails start as <path>: a group of keys, not a value (…), never quoting the value; an empty or blank value stays absent (P9-7). ValidationTests theory over the four keys asserts the failure and that the secret is not in it; red before, green after.
2 low design D, dependency-injection.md Docs said a bad configuration fails "each of its keyed services (the transport included)". A caller instance from UseSerde / UseTransport never consults the validated state and resolves fine (reproduced). Wording only: "every keyed service the package builds"; caller-supplied instances are returned as given. Dated correction (2026-10-13) in the design. Code unchanged: wrapping a caller instance in a factory would make the container dispose it (XCUT-22).
3 low (test quality) PrimaryHandlerLeaseTests.A_replaced_primary_is_the_callers_and_only_the_discarded_instance_is_released Only lease assertion was Assert.All(lease.Tracked, …), vacuous on an empty list; the test passed with lease.Track(handler) removed from NamedClient.cs. Now asserts exactly one tracked SocketsHttpHandler that is not the replacement, disposed after DisposeAsync, the replacement untouched. Mutation (Track removed) fails it plus three other lease tests; restored.

Rejected candidates

  • AddDexpaceClient twice with config.GetSection("X") throws (a new section instance each call): matches the design's same-instance rule; a usability point for the lead, not a defect.
  • Duration and integer grammar edge cases (PT1.5H, P0.5D, PT.5S, P5, overflow, signs, sub-tick fractions): accepted or rejected as CFG-7 / P9-29 say.
  • Failure messages echoing configured values: every path uses fixed rule text, except the numeric echo P9-28 allows.
  • HttpPipeline counted as a second keyed IAsyncHttpClient under SEAM-5: it is registered under its own type.
  • Lease vs transport disposal order: the transport borrows the chain and never disposes it, so the order is not safety-relevant.
  • Dependency audit (P9-3): policy record, closure and nuspec set are correct; the nuspec-lockstep negative control (skipped by the builder) was run by the reviewer and fires (depends on Dexpace.Sdk.Http.SystemNet 0.0.9, not its own lockstep version).

Builder deviations (all accepted by the review)

  1. No TransportSnapshot.cs; DexpaceClientState holds the transport options and pooled-connection lifetime.
  2. Per-name setup and validator are AddSingleton factories, not TryAddEnumerable (which would drop a second client's registrations, deduplicating by implementation type).
  3. KeyTable.Groups and the reader's Touched set (the drift guard needs both).
  4. BaseAddress and credential failures use fixed rule text rather than the design's UrlRedactor route (stricter: no value echoed).
  5. PrimaryHandlerLease.Track after disposal disposes the late handler at once (safer than R7's wording).
  6. The lease-ordering test uses a stand-in handler; no test would fail if the ordering line in NamedClient.cs were deleted. Not safety-relevant.
  7. The AddPolicy stage test uses PerCall/PerAttempt (Diagnostics refuses a second occupant).
  8. UseProxyFromEnvironment twice keeps the last call. Inconsistent with UseTransport/UseAuth, which throw on a different second install; left for the lead.
  9. Limited mutation evidence, recorded in the checklist.

Checks after the fixes

dotnet build Release 0 warnings · dotnet format --verify-no-changes clean · dotnet test --solution Dexpace.Sdk.sln 6 857 total, 0 failed, 22 skipped · housekeeping probe no drift. Before the fixes the builder also ran coverage (97.61%), the dependency audit, the reproducible pack, the AOT smoke, the tools solution and verify-structure, all green.

@Wahbeh-Mohammad
Wahbeh-Mohammad merged commit af603d2 into main Oct 13, 2026
5 checks passed
@Wahbeh-Mohammad
Wahbeh-Mohammad deleted the 96-phase-9-host-integration branch October 13, 2026 14:49
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 9: host integration (Dexpace.Sdk.Extensions.DependencyInjection)

1 participant