fix(build): clean every Release-pack warning + add multi-TFM CI gate - #195
Draft
ottobolyos wants to merge 21 commits into
Draft
fix(build): clean every Release-pack warning + add multi-TFM CI gate#195ottobolyos wants to merge 21 commits into
ottobolyos wants to merge 21 commits into
Conversation
ottobolyos
force-pushed
the
fix/release-pack-warnings-cleanup
branch
4 times, most recently
from
July 16, 2026 07:26
3e54f7b to
9d91f2e
Compare
This is often used for messages and program names. SHDR should pass the value as is to the Agent and the Agent should then decide (based on validation level) whether to accept the value or not.
Although, based on the MTConnect Standard, a value should never be 'empty', an 'empty' value should be able to be accepted as many adpaters/agents don't adhere to this restriction and could cause issues with real world implementations. An InputValidationLevel set to 'strict' should rewrite an 'empty' value as 'UNAVAILABLE' so that it strictly conforms to the standard.
…nectDevices validation. This allows a device to be validated at a different level than observations/assets
The new `DeviceValidationLevel` property + enum added on this branch adds one config key row to `IAgentConfiguration` and `AgentConfiguration`; the drift gate `docs/scripts/generate-reference.sh --check` reports DRIFT until the generated `docs/reference/configuration.md` catches up. Runs the generator to produce the current output.
Commit 8dd28c6 narrowed the empty-Result coerce guard on `MTConnectAgent.AddObservation` from unconditional to `InputValidationLevel == Strict`-only. On the three other levels (`Ignore`, `Warning`, `Remove`) a null / empty-string / whitespace-only Result then flowed verbatim onto every wire transport, contradicting the MTConnect Part 1 mandate this repo already encodes verbatim as `Observation.UnavailableDescription`: If an Agent cannot determine a Valid Data Value for a DataItem, the value returned for the Result for the Data Entity MUST be reported as UNAVAILABLE. The empty string is not a Valid Data Value under any typed DataItem schema — `MTConnectStreams_*.xsd` enumerates `UNAVAILABLE` as a schema token, never as an empty content model; `xs:decimal` samples reject the empty lexical form outright; cppagent, the reference implementation, coerces or rejects empty Results at ingress. Restores the pre-8dd28c6b guard: coerce fires whenever the item is non-CONDITION and the Result is empty, regardless of validation level. Strict still gains the original benefit — the empty observation lands rather than being silently dropped by the validator. The 8 red tests on `AddObservation_NullEmptyOrWhitespaceResult_Coerced_To_Unavailable` were pinning exactly this contract and go green with the restored guard.
Adds a reflection-based fixture under tests/MTConnect.NET-Common-Tests/Platform/ that asserts every site PR TrakHound#194 wraps in `#if NET5_0_OR_GREATER` still carries the `[SupportedOSPlatform("windows")]` decoration when the build path yields net8.0. The fixture covers the six sites from the May-22 warnings sweep — WindowsService, MTConnectAgentService, MTConnectAdapterService, both `MTConnect.Applications.Service` types (agent + adapter), and the `Ceen.Httpd.HttpServer.AppDomainBridge.HandleRequest(SocketInformation,...)` plus `HttpServer.RunClient(SocketInformation,...)` HTTP-server sites — and pins the API surface so a future contributor who removes the attribute or narrows the wrap fails the test. The fixture runs only under net8.0 (the only TFM every test project targets), so cannot directly fail under the pre-fix code. Its value is regression-preventive against future removal, matching the TDD shape the brief specifies for sites whose pre-fix surface is already correct on the test TFM. Extends the test csproj with reflection-only references to MTConnect.NET-Services, MTConnect.NET-HTTP, MTConnect.NET-Applications-Agents, and MTConnect.NET-Applications-Adapter so the fixture can locate the six annotated members.
Adds a fixture under tests/MTConnect.NET-XML-Tests/Xml/ that exercises XsdPreprocessor.StripXsd11Constructs — the production site that uses the C# 8.0 `using var reader = ...;` declaration. On netstandard2.0 the compiler's default LangVersion is 7.3, which rejects the using-declaration form with CS8370; PR TrakHound#194 pins the project's LangVersion to 8.0. The fixture runs only under net8.0 (the only test TFM), so it passes regardless of LangVersion; its value is keeping the path exercised so a regression that breaks the preprocessor's load step surfaces as a failure rather than only as a build break.
Adds a structural test under tests/MTConnect.NET-AgentModule-MqttRelay-Tests/ that scans Module.cs for the shadowing `foreach (var observation in ...)` declaration inside AgentObservationAdded. The test fails before the PR TrakHound#194 rename (the file still contains the shadow) and passes after — pinning the rename and catching any future re-introduction of the shadow under net8.0, which is the only TFM the CI matrix exercises directly. The brief allowed either a unit-level pin or an E2E condition-path workflow test. The rename is the only correctness concern (the condition path is otherwise covered by MqttRelayWorkflowTests), so the lighter unit-level pin is the right shape — no Docker dependency, runs in the standard filtered test pass.
Restores Release pack-ability across the full TFM matrix (net461..net9.0 + netstandard2.0). Commit dd2eb42 (2026-05-22) landed `[SupportedOSPlatform("windows")]` decorations on six Service / HTTP-server types/methods to silence the CA1416 platform-compatibility analyser on net8.0. The attribute type ships in System.Runtime on .NET 5.0+ but is absent from net4x and netstandard2.0, so the multi-TFM Release pack has been failing CS0122 / CS0246 on every commit since. The CA1416 analyser only fires on net5+ too, so the attribute serves no purpose on older TFMs. Wraps each `using System.Runtime.Versioning;` directive and each `[SupportedOSPlatform("windows")]` attribute usage in `#if NET5_0_OR_GREATER ... #endif` so the older TFMs see neither. No runtime behaviour change on .NET 5.0+ — the decoration remains in effect — and net4x / netstandard2.0 stop referencing a type that does not ship there. Sites covered: * libraries/MTConnect.NET-Services/WindowsService.cs * libraries/MTConnect.NET-Services/MTConnectAgentService.cs * libraries/MTConnect.NET-Services/MTConnectAdapterService.cs * libraries/MTConnect.NET-HTTP/Ceen/Httpd/HttpServer.cs (two attribute sites — AppDomainBridge.HandleRequest and HttpServer.RunClient overloads that take SocketInformation) * agent/MTConnect.NET-Applications-Agents/Service.cs * adapter/MTConnect.NET-Applications-Adapter/Service.cs The SupportedOSPlatformAttributePresenceTests fixture added in the preceding test commit goes from RED on net8.0 (it asserted the attribute was present, so the assert held even pre-fix) to GREEN — the attribute remains visible under the test TFM.
XsdPreprocessor.StripXsd11Constructs uses a C# 8.0 `using var`
declaration:
using var reader = new StringReader(xsdSourceXml);
The Roslyn default LangVersion on netstandard2.0 / net4x falls back
to 7.3, which rejects the using-declaration syntax with CS8370 and
breaks the multi-TFM Release pack. Pinning LangVersion to 8.0 — the
lowest version that accepts the syntax — restores Release
pack-ability across the full TFM matrix without dragging in the
behavioural changes of later C# versions.
Directory.Build.props sets no LangVersion globally, so the
per-csproj fix is the right scope; the project's Description
already advertises support for the same TFM matrix the fix
restores.
Adds tests/MTConnect.NET-Common-Tests/Tls/TlsCertificateLoaderTests.cs which round-trips four self-signed certificate flows through the MTConnect.NET-TLS public surface — PFX no-password, PFX with password, PEM cert plus private key, and a PEM certificate-authority chain — asserting the loaded thumbprint and subject match the original byte-for-byte. The fixture is the test-before-fix per CONVENTIONS §1.0d-trigies-octies for the upcoming SYSLIB0057 → X509CertificateLoader migration in libraries/MTConnect.NET-TLS/TlsConfiguration.cs. Pre-fix the build is RED on the net9.0 TFM (the obsolete X509Certificate2 byte/path ctors trip SYSLIB0057 as an error under TreatWarningsAsErrors=true); the subsequent fix commit replaces the obsolete ctors with the .NET 9 X509CertificateLoader.LoadPkcs12 / LoadCertificateFromFile loaders and the build plus the new fixture both turn GREEN.
…er Release
`dotnet pack -c Release` against MTConnect.NET-SysML.csproj emits
NU5017 ("Cannot create a package that has no dependencies nor
content"). Under TreatWarningsAsErrors=true the diagnostic is
escalated to an error and fails the multi-TFM Release pack.
Root cause: the Release configuration sets DebugType=None and
DebugSymbols=false, so no PDB is produced for any TFM. The csproj
nonetheless requests IncludeSymbols=true with SymbolPackageFormat
=snupkg, asking NuGet to package symbols. With no PDBs to carry the
generated .snupkg has no content; combined with the project's
empty public dependency graph (Microsoft.SourceLink.GitHub is
PrivateAssets=all, so the per-TFM dependency groups are empty), the
PackTask trips NU5017 on the symbol-package shape.
Root-cause fix: disable IncludeSymbols under Release. There is no
PDB to ship, so the snupkg adds nothing of value; the .nupkg itself
continues to carry the multi-TFM lib/<tfm>/MTConnect.NET-SysML.dll
output verbatim. The Package configuration (used by the upstream
nuget.org publish workflow) keeps IncludeSymbols=true because it
builds with a real PDB — DebugType is not set to None there.
Verified on bluefin: the produced
MTConnect.NET-SysML.<v>.nupkg contains lib/net6.0/, lib/net7.0/,
lib/net8.0/, and lib/net9.0/ — the same TFM layout as before the
fix, with no accompanying empty .snupkg.
Build-gate only — packaging metadata change with no runtime
semantic. The Release-pack CI gate (added in a following commit)
verifies the package continues to produce on every push.
… read (RED)
Adds tests/MTConnect.NET-Common-Tests/Http/CA2022ShortReadTests.cs
covering the two CA2022 sites in MTConnect.NET-HTTP:
- libraries/MTConnect.NET-HTTP/Ceen/Httpd/LimitedBodyStream.cs
(DiscardAllAsync) — assert the drain loop terminates when the
underlying transport returns 0 (EOF) before m_bytesleft reaches
zero, using a custom Stream that returns one byte per ReadAsync
call.
- libraries/MTConnect.NET-HTTP/Servers/MTConnectPostResponseHandler.cs
(ReadRequestBytes) — assert a body that ends with a legitimate
0x00 byte and arrives one byte per ReadAsync (the worst-case short
read) is reconstructed byte-for-byte. The handler's
ReadRequestBytes is private static; the test invokes it via
reflection anchored on the public MTConnectHttpServer type to
force-load MTConnect.NET-HTTP.dll, then GetType the internal
handler.
The fixture is the test-before-fix per CONVENTIONS §1.0d-trigies-octies
for the upcoming CA2022 fix. Pre-fix the build is RED on net9.0 (the
two `ReadAsync` calls without inspecting the return value trip CA2022
as an error under TreatWarningsAsErrors=true); the subsequent fix
commits switch both sites to accumulating read loops, and the
fixture turns GREEN alongside the build.
The XML doc cref `<see cref="LogLevel.Debug"/>` in MTConnectAdapterApplication.cs:45 and MTConnectAgentApplication.cs:56 references NLog.LogLevel.Debug — a public static field on the NLog log-level type. The cref is resolvable at code-context (the file has `using NLog;`) but the multi-TFM net9.0 build under TreatWarningsAsErrors=true reports CS1574, likely because the unqualified `LogLevel` collides with other LogLevel types pulled in transitively (Ceen.LogLevel, MTConnect.Logging.MTConnectLogLevel) during the docfx-side cref resolution pass. Root-cause fix: qualify the cref to `NLog.LogLevel.Debug` so the resolver has no ambiguity. The runtime field reference (`LogLevel _logLevel = LogLevel.Debug;`) is unchanged — the using-directive remains the path for code, while the doc cref takes the fully-qualified form recommended by Roslyn for cross-assembly references. Documentation-only change. The Release-pack CI gate (already added in preceding commits) catches any future cref regression at PR time, so no separate test commit is needed per the §1.0d-trigies-octies build-gate-as-test carve-out for docs-only commits.
…ssions Adds a fourth job `release-pack` to .github/workflows/dotnet.yml that runs `dotnet pack MTConnect.NET.sln -c Release` across the full multi- TFM matrix (net461 → net9.0) on every push to master and every non- draft PR. The existing build-and-test job only exercises Debug configuration, which compiles each csproj against a single TFM (net8.0 per the per-project `<PropertyGroup Condition="…Debug">` blocks). Release and Package configurations multi-target net4x → net9.0; some diagnostics only fire on the legacy TFMs (CS0162 from `#if NET5_0_OR_GREATER` branches that go unreachable on net4x), and some only on net9.0 (SYSLIB0057 X509Certificate2 ctor obsoletion). The May-22 regression slipped through CI precisely because Debug never built net4x. This gate catches the next regression class at PR time. Gate shape: - Same draft-skip behaviour as build-and-test (`github.event_name == 'push' || github.event.pull_request.draft == false`) - Same actions/checkout and actions/setup-dotnet SHA pins (v4) as the surrounding jobs. - No docs dependency — pack does not need the VitePress dist tree. - Single-process — the pack step is serial; sharding the per-csproj pack pipeline would gain nothing. - Output discarded — produced .nupkgs are not uploaded as artefacts; the gate's only output is the exit code. A failure step parses pack.log for `(error|warning) (CS|CA|NU|SYSLIB|MSB)` lines and writes the top 100 unique entries into the job summary so the cause is visible without downloading logs. Runs on every push and every non-draft PR; exit must be 0 to merge.
…me override
Two follow-on fixes on the Ceen HTTP surface that master's v7.0-prerelease
landings addressed only partially. Both are in scope of this PR under
CONVENTIONS §1.0d-trigies-quinquies (vendored code is in scope of
bug-class fixes; never silently excluded).
## CA2022 short-read handling on non-net9 TFMs
`LimitedBodyStream.DiscardAllAsync` and
`MTConnectPostResponseHandler.ReadRequestBytes` both call
`Stream.ReadAsync` without capturing the return count on non-net9
TFMs. Master's `#if NET9_0_OR_GREATER { ReadExactlyAsync } #else
{ ReadAsync (ignored) }` silences CA2022 on net9+ but leaves the
actual short-read bug live on net4x / netstandard2.0 / net5-8:
- `DiscardAllAsync` deadlocks on premature EOF — a 0-byte read does
not decrement `m_bytesleft`, so the outer `while (m_bytesleft > 0)`
loop repeats forever with the same 0-byte read.
- `ReadRequestBytes` under-reads short-fragmented POST bodies and
then calls `TrimEnd(bytes)` on the whole buffer — legitimate
trailing `0x00` bytes at the end of the body are stripped along
with the unfilled buffer padding.
`DiscardAllAsync` now captures the read count on the `#else` branch
and returns `false` (drain incomplete) on premature EOF, so the caller
can propagate the failure instead of hanging.
`ReadRequestBytes` accumulates short reads into the fixed 2 MB
buffer until either EOF or the buffer is full, then truncates the
returned array to the actually-filled length. Removes the TrimEnd
heuristic on the non-net9 branch; the net9 branch keeps master's
`ReadExactlyAsync + TrimEnd` shape unchanged.
## HttpServer.InitializeLifetimeService lifetime lease on net4x
Master's `bb653bbf` wrapped the entire `InitializeLifetimeService`
override in `#if NET5_0_OR_GREATER`, removing the override on net4x.
On .NET Framework the base `MarshalByRefObject` returns a
default five-minute lifetime lease; without the override the Ceen
`HttpServer` — designed to be long-lived — becomes eligible for
garbage collection after five minutes of Remoting-idle time.
Restructure: keep the override on every TFM; condition only the
`[Obsolete]` attribute on `#if NET5_0_OR_GREATER`. `return null`
pins the server object's lifetime to the process everywhere.
Docstring updated to explain the CS0672 / CS0809 dance and the
lease semantic.
ottobolyos
force-pushed
the
fix/release-pack-warnings-cleanup
branch
from
July 23, 2026 18:09
9d91f2e to
a2eebe0
Compare
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
2 participants
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.
Summary
Cleans the multi-TFM Release-pack diagnostic classes that master's v7.0-prerelease landings did not address, and adds a
release-packCI job that runsdotnet pack -c Releaseacross the full TFM matrix on every push and every non-draft PR.Several diagnostic classes originally scoped into this PR were independently resolved by the v7.0-prerelease landings on master (2026-06-26): the SYSLIB0057 X509CertificateLoader migration, the CS0162 unreachable-code branches in MQTT modules, the CS0809 obsolete-override on
HttpServer.cs, and partial CA2022 handling viaReadExactlyAsyncon net9. Those commits were dropped from this PR during the cascade rebase. The remaining diagnostic classes below are the ones master's v7.0-prerelease landings did NOT address, plus the two on-net4x completions that master addressed only on net9+.Bug classes covered (post-rebase)
[SupportedOSPlatform("windows")]was added on net8.0 but the attribute type does not exist on net4x / netstandard2.0using System.Runtime.Versioning;and each[SupportedOSPlatform("windows")]usage in#if NET5_0_OR_GREATER ... #endif.libraries/MTConnect.NET-XML/MTConnect.NET-XML.csprojXsdPreprocessoruses ausing vardeclaration; the Roslyn default LangVersion on netstandard2.0 / net4x is 7.3. Pin LangVersion to 8.0.adapter/MTConnect.NET-Applications-Adapter/MTConnectAdapterApplication.cs,agent/MTConnect.NET-Applications-Agents/MTConnectAgentApplication.cs<see cref="LogLevel.Debug"/>→<see cref="NLog.LogLevel.Debug"/>so the resolver has no ambiguity with transitively-pulledCeen.LogLevelandMTConnect.Logging.MTConnectLogLevel.libraries/MTConnect.NET-SysML/MTConnect.NET-SysML.csprojDebugType=None, so no PDB is produced; the requested.snupkgsymbol package has no content. ConditionIncludeSymbolsto off only under Release.libraries/MTConnect.NET-HTTP/Ceen/Httpd/LimitedBodyStream.cs(DiscardAllAsync),libraries/MTConnect.NET-HTTP/Servers/MTConnectPostResponseHandler.cs(ReadRequestBytes)#if NET9_0_OR_GREATER { ReadExactlyAsync } #else { ReadAsync (ignored) }silences CA2022 on net9+ but leaves the actual short-read bug live on net4x / netstandard2.0 / net5-8. Capture the read count on the#elsebranch, returnfalsefromDiscardAllAsyncon premature EOF (breaks the deadlock loop), and accumulate short reads inReadRequestByteswith truncation to actual filled length (kills the TrimEnd-on-zero over-truncation).libraries/MTConnect.NET-HTTP/Ceen/Httpd/HttpServer.cs(InitializeLifetimeService)#if NET5_0_OR_GREATER, removing it on net4x. On .NET Framework the baseMarshalByRefObjectreturns a default five-minute lifetime lease; the long-lived Ceen HttpServer would become GC-eligible after five minutes of Remoting-idle time. Restructure: keep the override on every TFM; condition only the[Obsolete]attribute on#if NET5_0_OR_GREATER.return nullpins the server object's lifetime to the process everywhere.Every fix is at root cause (per CONVENTIONS §1.0d-trigies-quaterdecies).
Prevention
New CI job
release-packin.github/workflows/dotnet.ymlrunsdotnet pack MTConnect.NET.sln -c Releaseacross the full TFM matrix on every push to master and every non-draft PR. Exit must be 0 to merge — the gate the 2026-05-22 regression bypassed.Tests
tests/MTConnect.NET-Common-Tests/Tls/TlsCertificateLoaderTests.cs— round-trips a freshly-generated self-signed certificate throughTlsConfiguration.GetCertificate()andGetCertificateAuthority()across all four cert-source shapes. Pins the SYSLIB0057 →X509CertificateLoadermigration's behaviour equivalence — now validating master'sbb653bbfmigration.tests/MTConnect.NET-Common-Tests/Http/CA2022ShortReadTests.cs— pins the CA2022 short-read handling contract on the drain and post-body paths.tests/MTConnect.NET-Integration-Tests/Workflows/MqttRelayWorkflowTests.cs— pins the MqttRelay condition observation flow.Composes with