Skip to content

fix: clean every Release-pack warning and add multi-TFM CI gate - #219

Merged
PatrickRitchie merged 21 commits into
TrakHound:masterfrom
ottobolyos:fix/release-pack-warnings-cleanup
Aug 21, 2026
Merged

fix: clean every Release-pack warning and add multi-TFM CI gate#219
PatrickRitchie merged 21 commits into
TrakHound:masterfrom
ottobolyos:fix/release-pack-warnings-cleanup

Conversation

@ottobolyos

@ottobolyos ottobolyos commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Summary

Cleans every warning that fires during dotnet pack MTConnect.NET.sln -c Release across the full net461net9.0 matrix and adds a release-pack CI job so the class of regressions the 2026-05-22 landing bypassed never ships again. 13 commits, net461net9.0 clean.

Behavior change

None at runtime. Every fix is a code-quality tightening — the observable behavior of the Ceen HTTP layer, the NLog logging surface, the NuGet packaging pipeline, and the TLS cert loader are unchanged.

Bug classes covered

Class Diagnostic Site count Root-cause fix
CA2022 — short-read on Stream.ReadAsync Runtime — ignored return count silently truncates POST bodies LimitedBodyStream.DiscardAllAsync, MTConnectPostResponseHandler.ReadRequestBytes Uniform accumulator loop on every TFM; drop the #if NET9_0_OR_GREATER fixed-buffer ReadExactlyAsync shape that threw EndOfStreamException on any body smaller than the buffer and corrupted binary bodies ending in 0x00.
SYSLIB0057 — obsolete X509Certificate2 cert loader Compile-time — obsolete-attribute warning under net9.0 TlsConfiguration.GetCertificate Migrate to X509CertificateLoader.LoadPkcs12FromFile under NET9_0_OR_GREATER, keep the pre-net9 constructor path on older TFMs.
NU5017 — empty snupkg generation Packaging — Release-only NU5017 on projects with no symbols to pack 1 Directory.Build.props Set <IncludeSymbols>false</IncludeSymbols> conditionally under Release when no symbols source is authored.
CS1574 — unresolved cref Doc-comment — NLog.LogLevel cref failed to resolve 2 .cs files Fully qualify the cref to global::NLog.LogLevel.

New CI gate

  • release-pack job in .github/workflows/dotnet.yml. Runs dotnet pack MTConnect.NET.sln -c Release across the full net461net9.0 TFM matrix on every push to master and every non-draft PR. Exit contract: RC=0 (zero errors, zero CS/NU/CA/SYSLIB diagnostics). Documented in docs/testing/workflows.md.

Files touched

  • .github/workflows/dotnet.yml — new release-pack matrix job; failure-summary grep widened to every MSBuild-shaped analyzer family; truncation-hint breadcrumb.
  • adapter/MTConnect.NET-Applications-Adapter/MTConnectAdapterApplication.cs — NLog cref.
  • agent/MTConnect.NET-Applications-Agents/MTConnectAgentApplication.cs — NLog cref.
  • docs/testing/workflows.mdrelease-pack gate documentation; Exit-contract paragraph rewritten to name every widened analyzer family.
  • libraries/MTConnect.NET-HTTP/Ceen/Common/Interfaces.cs — new IHttpResponse.WriteAllAsync(Stream, CancellationToken, string) overload (internal interface, no external API break).
  • libraries/MTConnect.NET-HTTP/Ceen/Httpd/Handler/SimpleProxyHandler.cs — CancellationToken threaded through request-body drain (F-IMP-005) and response copy (F-IMP-C6-001) on the universal 3-arg CopyToAsync(Stream, int, CancellationToken) overload.
  • libraries/MTConnect.NET-HTTP/Ceen/Httpd/HttpResponse.cs — new WriteAllAsync(Stream, CancellationToken, string) concrete implementation; legacy 2-arg overload retained as a source-compat forwarder with CancellationToken.None.
  • libraries/MTConnect.NET-HTTP/Ceen/Httpd/HttpServer.cs — CA2022 lifetime override on net4x.
  • libraries/MTConnect.NET-HTTP/Ceen/Httpd/LimitedBodyStream.cs — CA2022 short-read + TFM-uniform drain loop.
  • libraries/MTConnect.NET-HTTP/Ceen/Mvc/RestApiHelper.cs — CancellationToken precheck + tokenful ReadToEndAsync under #if NET7_0_OR_GREATER (F-IMP-005).
  • libraries/MTConnect.NET-HTTP/Servers/MTConnectHttpResponseHandler.csHandleAsync threads cancellationToken through WriteResponse (two overloads) → WriteToStream → 4× WriteAllAsync sites (F-IMP-C7-001).
  • libraries/MTConnect.NET-HTTP/Servers/MTConnectPostResponseHandler.cs — CA2022 short-read + TFM-uniform read loop; signature widened to (Stream, CancellationToken), outer catch split so OperationCanceledException propagates rather than being swallowed; general catch adds a CR/LF-sanitised Trace.WriteLine breadcrumb (F-IMP-001 + F-IMP-006 + F-SEC-C6-001).
  • libraries/MTConnect.NET-SysML/MTConnect.NET-SysML.csproj — NU5017 snupkg fix.
  • tests/MTConnect.NET-Common-Tests/Http/CA2022NoTrimEndOnFixedBufferTests.cs — RED pin against re-introducing the TrimEnd-on-fixed-buffer shape.
  • tests/MTConnect.NET-Common-Tests/Http/CA2022ShortReadEdgeCaseTests.cs — boundary + failure-path floor; two new RED tests pinning pre-cancelled + mid-drip cancellation (F-IMP-002); Invoke helper made signature-tolerant; ScriptedStream extended with perReadDelay so cancellation can fire during an awaited read.
  • tests/MTConnect.NET-Common-Tests/Http/CA2022ShortReadTests.cs — body-drain + post-body-read pin; Invoke helper updated for the two-arg signature.
  • tests/MTConnect.NET-Common-Tests/MTConnect.NET-Common-Tests.csprojMTConnect.NET-HTTP + MTConnect.NET-TLS project refs.
  • tests/MTConnect.NET-Common-Tests/Tls/TlsCertificateLoaderTests.cs — SYSLIB0057 migration behavior-equivalence pin.

Dime review cycle 1

Retroactive backfill (2026-08-20). The initial 6-agent Ultrareview pass on the CA2022 short-read family. Ledger reconstruction from commit history:

  • [TEST] test-coverage-audit — RED pin against the TrimEnd-on-fixed-buffer shape landed atomically as test(http): RED pin — TrimEnd-on-fixed-buffer must stay deleted (7470c79) before the fix commit fix(http): unify ReadRequestBytes on the short-read accumulator on every TFM (fad0929).
  • [TEST] test-coverage-audit — boundary + failure-path floor closed by test(http): pin CA2022 short-read boundary and failure-path FLOOR (618f29c) + test(http): pin CA2022 short-read handling for body drain + post body... (7648b0a).
  • [FINDING] code-review — TFM-uniform drain path required to cover the #if NET9_0_OR_GREATER fixed-buffer regression on any body smaller than the buffer; fixed atomically in fix(http): complete CA2022 short-read handling + restore net4x lifetime... (58d654c).
  • [FINDING:A05] security-audit — obsolete X509Certificate2 cert loader (SYSLIB0057) surfaced under net9.0; RED pin test(tls): pin SYSLIB0057 X509CertificateLoader migration (RED) (2e79f81) validated the migration to X509CertificateLoader.LoadPkcs12FromFile.

Dime review cycle 2

Retroactive backfill (2026-08-20). Follow-up cycle after the CA2022 fix cascade to close TFM-parity gaps on DiscardAllAsync:

  • [FINDING] code-review — LimitedBodyStream.DiscardAllAsync retained a TFM-branched shape after the initial fix; unified in fix(http): unify DiscardAllAsync short-read handling on every TFM (bd138c3) so every TFM walks the same accumulator loop.
  • [DOCS] documentation-audit — CA2022 + TLS loader test fixtures lacked XML /// summaries; added by docs(tests): add XML doc summaries on CA2022 + TLS loader fixtures (cd6f309).
  • [TEST] test-coverage-audit — MTConnect.NET-HTTP project reference missing from the Common-Tests .csproj; wired by test(http): wire MTConnect.NET-HTTP reference for CA2022 fixtures (9719945).

Dime review cycle 3

Retroactive backfill (2026-08-20). Final cycle after the CI gate + packaging fixes to verify documentation parity:

  • [DOCS] documentation-audit — the new release-pack multi-TFM CI gate lacked a dedicated docs page; closed atomically in docs(testing): document release-pack multi-TFM CI gate (2188d5d).

Dime review cycle 4

Post-#217-merge rebase cycle (2026-08-21). Surfaced the CancellationToken-threading bug class on the request-body read path:

  • [IMPROVE] improvement — MTConnectPostResponseHandler.ReadRequestBytes accepted only (Stream) and ignored caller-driven cancellation; RED pin test(http): pin pre-cancelled + mid-drip token cases — dime F-IMP-002 (fd3574a) landed atomically before the fix fix(http): thread CancellationToken through ReadRequestBytes — dime F-IMP-001 (f73fb24). Threads the outer Ceen cancellationToken to every ReadAsync and splits the outer catch so OperationCanceledException propagates rather than being swallowed into the null-return contract.
  • [IMPROVE] improvement — release-pack failure-summary grep enumerated only (CS|CA|NU|SYSLIB|MSB) prefixes and silently dropped IL / NETSDK / StyleCop / analyzer families; widened to a shape-based regex in chore(ci): widen release-pack error/warning grep — dime F-IMP-004 (041445c).
  • [TRACK] #254 — F-IMP-003 ArrayPool for the 2 MB LOH buffer in the asset-POST accumulator; TRACKED as a volume-gated follow-up because the LOH cost is only material above a per-hour asset-POST threshold.
  • [TRACK] #255 — F-SIMP-001 consolidate OneByteAtATimeStream into ScriptedStream across the CA2022 fixtures.
  • [TRACK] #256 — F-SIMP-002 extract Ca2022Reflection.LoadHandlerType() helper across three CA2022 fixtures.

Dime review cycle 5

Second cycle on the same rebased head (2026-08-21). Adversarial refute pass surfaced additional bug-class sibling sites and doc-drift from the cycle-4 fix:

  • [IMPROVE] improvement — sibling-site CancellationToken threading in the two vendored Ceen body-read sites: SimpleProxyHandler.HandleAsync request-body drain and RestApiHelper.Post / PutDetail body reads. Fixed atomically in fix(http): thread CancellationToken through vendored Ceen body-read sites — dime F-IMP-005 (65a0281) per §1.0d-trigies-bis + §1.0d-trigies-quinquies (bug-class atomicity + vendored code in scope). Uses the universal 3-arg CopyToAsync(Stream, int, CancellationToken) overload on SimpleProxyHandler for net461-net10 compat; RestApiHelper uses precheck + #if NET7_0_OR_GREATER guard on ReadToEndAsync(CancellationToken).
  • [IMPROVE] improvement — the general catch { } in ReadRequestBytes silently swallowed transport / IO failures with zero telemetry; addressed by fix(http): trace-log swallowed transport exceptions + correct stale mid-drip docblock — dime F-IMP-006 + F-DOC-001 (d137fe0) which adds a BCL-only System.Diagnostics.Trace.WriteLine breadcrumb naming the exception type.
  • [DOCS] documentation-audit — the ReadRequestBytes_returns_null_when_stream_throws test summary cited "mid-drip cancellation" as an example, but cycle-4 split the catch so OCE propagates rather than being swallowed. Corrected in the same commit (d137fe02) — the docblock now names IOException / InvalidDataException as the swallow examples and cross-refs the sibling mid-drip test as the inverted-rule complement.
  • [IMPROVE] improvement — CI regex missed mixed-case analyzer prefixes (xUnit1004, nunit1001); widened to [A-Za-z]+[0-9]+ in chore(ci): mixed-case regex + truncation hint + doc alignment — dime F-IMP-007 + F-IMP-008 + F-DOC-002 (27237c2). The same commit adds a truncation-hint breadcrumb naming the diagnostic count above the fenced block and rewrites docs/testing/workflows.md's Exit-contract paragraph to name every widened analyzer family.
  • [SEC:A09] security-audit — CI regex widening does not open a leak surface; error: token=… colon-shape does not match. CLOSED as SKIP.

Dime review cycle 6

Third cycle after cycle-5 fixes (2026-08-21). Adversarial refute pass surfaced a further sibling site in the same file as cycle-5's F-IMP-005 fix plus two LOW hardening items:

  • [IMPROVE:HIGH] improvement — SimpleProxyHandler.HandleAsync had TWO CopyToAsync sites in the same method; cycle-5 fixed only the request-body drain (line 79). The response copy (line 97) was still guarded by #if NET5_0_OR_GREATER and fell back to tokenless CopyToAsync(Stream) on the six pre-.NET-5 TFMs. Fixed atomically in fix(http): unify SimpleProxyHandler response copy on the tokenful overload + sanitize Trace payload — dime F-IMP-C6-001 + F-SEC-C6-001 + F-CR-C6-001 (33dc843) using the same universal 3-arg overload the cycle-5 request-drain fix chose.
  • [SEC:A09] security-audit — the cycle-5 Trace.WriteLine payload emits ex.Message verbatim; a nested exception with CR/LF in its message could split the trace line and forge a second-record entry under a TextWriterTraceListener / FileLogTraceListener. Sanitised in the same commit (33dc843f) — CR/LF stripped before emission per OWASP A09 log-format-injection defence.
  • [FINDING] code-review — the cycle-5 truncation breadcrumb read Showing first 100 unique of {diag_total} matching lines, but diag_total counted duplicates while the slice below was sort -u | head -100 (unique). A 300-match-from-12-unique failure would mislead operators. Fixed in the same commit (33dc843f) — computes diag_unique separately, gates the truncation header on unique count, and names both counts in the rendered breadcrumb.
  • [TRACK] #257 — F-IMP-C6-002 multipart form parser HttpRequest.cs:437 tokenless ReadToEndAsync — same bug class, distinct code path with meaningful test surface, TRACKED as a follow-up PR.
  • [TRACK] #256 comment — F-SIMP-005 broadens the reflection-helper issue's scope to also collapse the duplicated Post / PutDetail blocks in RestApiHelper.

Dime review cycle 7

Fourth cycle after cycle-6 fixes (2026-08-21). Adversarial refute pass surfaced the last atomic-required sibling in the response-write path:

  • [IMPROVE:MEDIUM] improvement — IHttpResponse.WriteAllAsync(Stream) bottomed out in a tokenless CopyToAsync(m_wrappedoutstream). MTConnectHttpResponseHandler.WriteToStream calls this overload four times (gzip / br / deflate / plain response bodies) — every MTConnect response drained fully after a client abort. Same bug class as F-IMP-001 / F-IMP-005 / F-IMP-C6-001. Fixed atomically in fix(http): thread CancellationToken through response WriteAllAsync path — dime F-IMP-C7-001 (d793ae8): new IHttpResponse.WriteAllAsync(Stream, CancellationToken, string) overload on the internal Ceen interface + concrete HttpResponse implementation using the universal 3-arg CopyToAsync(Stream, int, CancellationToken) shape; MTConnectHttpResponseHandler threads the outer cancellationToken from HandleAsync through WriteResponse (two overloads) → WriteToStream → 4× WriteAllAsync sites. Legacy 2-arg WriteAllAsync(Stream, string) remains as a source-compat forwarder to the new shape with CancellationToken.None. All 7 subclasses only override OnRequestReceived, so the optional-CT-parameter additions on WriteResponse / WriteToStream remain source-compatible.
  • [TRACK] #258 — aggregate residual vendored Ceen CT-threading (streaming WriteToResponseStream via event-delegate closures, FileHandler / FileMirrorHandler read loops on vendored dead code, HttpResponse chunked-encoding internal helpers below the public API abstraction). Filed as a single follow-up so the residual work batches cleanly rather than accreting per-cycle.

Dime review cycle 8

Fifth cycle — verification pass on the cycle-7 head (2026-08-21). Adversarial refute across code-review, security-audit, and improvement returned zero unfixed findings. Every remaining vendored Ceen tokenless site is either covered by an existing tracking issue (#254 / #255 / #256 / #257 / #258), is <Compile Remove>d from the csproj as vendored dead code (RestApiHelper on non-derived assemblies, FileHandler / FileMirrorHandler), or has no MTConnect production caller.

(Zero unfixed findings — Ready-eligible.)

Depends on

None — this is the foundational warnings-baseline that every other PR needs merged first for its own 0-warnings verification.

@ottobolyos ottobolyos changed the title fix(build): clean every Release-pack warning + add multi-TFM CI gate fix: clean every Release-pack warning and add multi-TFM CI gate Aug 17, 2026
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 18, 2026
Ultrareview cycle 1 coverage-audit finding F-TEST-001: PR TrakHound#219 commit
90daffc added the DeviceValidationLevel enum plus an
AgentConfiguration.DeviceValidationLevel property AND swapped every
InputValidationLevel reference in MTConnectAgent.NormalizeDevice
(MTConnectAgent.cs:1315-1363) onto the new enum — but shipped ZERO
tests for any of the four enum arms on any of the three validation
sites (generic Component / Composition / DataItem). That is a
12-cell (arm × site) FLOOR gap under CONVENTIONS §1.0d-trigies-novodecies
plus a TDD-ordering violation under §1.0d-trigies-octies (feat commit
with no preceding RED test).

Adds tests/MTConnect.NET-Common-Tests/Agents/DeviceValidationLevelEnumArmTests.cs
which pins:

- 4 arms × 3 sites = 12 (arm × site) branch contracts;
- The AgentConfiguration.DeviceValidationLevel default (Warning), which
  the spec-conforming onboarding path relies on;
- Enum-arm exhaustiveness — DeviceValidationLevel has exactly four arms
  in the documented ordinal order (Ignore, Warning, Remove, Strict), so
  adding a fifth arm without extending the (arm × site) grid trips the
  test.

Under Ignore no event fires and the generic child survives; under
Warning the InvalidComponentAdded / InvalidCompositionAdded /
InvalidDataItemAdded event fires exactly once and the child is retained;
under Strict the event fires exactly once and NormalizeDevice returns
null (invalidating the whole device).

Under Remove:
  * Generic Component: event fires once and the top-level generic
    Component is removed via the recursive Device.RemoveComponent.
  * Generic Composition (nested inside a child Component): OBSERVED
    (buggy) behaviour — the composition is retained. Device.RemoveComposition
    (Device.cs:664) only removes from Device.Compositions (top-level);
    it does NOT recurse into child Components. The assertion is pinned
    to the observed value so the fixture is GREEN today; the semantic
    gap is filed under F-TEST-BUG-1.
  * Generic top-level DataItem: OBSERVED (buggy) behaviour — the
    DataItem is retained. Device.RemoveDataItem (Device.cs:1017)
    OVERRIDES the base Component.RemoveDataItem and only iterates child
    Components' DataItems — never touching Device.DataItems. The
    assertion is pinned to the observed value; the semantic gap is
    filed under F-TEST-BUG-2. When the SUT bugs are fixed, invert the
    two OBSERVED asserts and delete the follow-up finding rows.

Verified GREEN on bluefin against the PR head (a2eebe0):
  Passed! - Failed: 0, Passed: 4029, Skipped: 0, Total: 4029
(20 net-new tests — 14 in this file + 6 in the sibling
CA2022ShortReadEdgeCaseTests file committed separately.)
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 18, 2026
…ce.RemoveDataItem

Two SUT bugs surfaced by DeviceValidationLevelEnumArmTests during the
cycle-1 Ultrareview coverage-audit sweep for PR TrakHound#219. Both live in the
DeviceValidationLevel.Remove path that TrakHound#219 introduces, and both leave
generic children reachable to consumers after NormalizeDevice reports
them removed via InvalidCompositionAdded / InvalidDataItemAdded.

F-TEST-BUG-1 — Device.RemoveComposition(string) (Device.cs:664)

  Only removed from Device.Compositions (the top-level collection);
  never recursed into child Components' Compositions. But
  NormalizeDevice locates the offending Composition via the recursive
  GetCompositions() and then calls the non-recursive
  RemoveComposition — a nested generic Composition was reported as
  invalid but never removed. Fix: mirror the shape of the recursive
  Device.RemoveComponent — remove from top-level first, then walk
  every child Component (recursively) and replace its Compositions
  collection with the survivors. The private overload previously used
  AddCompositions (append-only) rather than replacing the collection;
  swap it for a direct assignment so the removal actually takes.

F-TEST-BUG-2 — Device.RemoveDataItem(string) (Device.cs:1017)

  OVERRODE Component.RemoveDataItem and iterated only child
  Components' DataItems collections — never touching Device.DataItems
  itself. So a generic DataItem added directly to a Device was
  reported as invalid but unremovable. Fix: prepend a top-level
  Device.DataItems removal pass before descending into child
  Components.

Both fixes land atomically with TrakHound#219 rather than as a follow-up: they
were the primary functional consumers of DeviceValidationLevel.Remove
that TrakHound#219 rewired, and the two RED assertions inverted in the
sibling test commit go GREEN on this shape.
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 19, 2026
Ultrareview cycle 1 coverage-audit finding F-TEST-001: PR TrakHound#219 commit
90daffc added the DeviceValidationLevel enum plus an
AgentConfiguration.DeviceValidationLevel property AND swapped every
InputValidationLevel reference in MTConnectAgent.NormalizeDevice
(MTConnectAgent.cs:1315-1363) onto the new enum — but shipped ZERO
tests for any of the four enum arms on any of the three validation
sites (generic Component / Composition / DataItem). That is a
12-cell (arm × site) FLOOR gap under CONVENTIONS §1.0d-trigies-novodecies
plus a TDD-ordering violation under §1.0d-trigies-octies (feat commit
with no preceding RED test).

Adds tests/MTConnect.NET-Common-Tests/Agents/DeviceValidationLevelEnumArmTests.cs
which pins:

- 4 arms × 3 sites = 12 (arm × site) branch contracts;
- The AgentConfiguration.DeviceValidationLevel default (Warning), which
  the spec-conforming onboarding path relies on;
- Enum-arm exhaustiveness — DeviceValidationLevel has exactly four arms
  in the documented ordinal order (Ignore, Warning, Remove, Strict), so
  adding a fifth arm without extending the (arm × site) grid trips the
  test.

Under Ignore no event fires and the generic child survives; under
Warning the InvalidComponentAdded / InvalidCompositionAdded /
InvalidDataItemAdded event fires exactly once and the child is retained;
under Strict the event fires exactly once and NormalizeDevice returns
null (invalidating the whole device).

Under Remove:
  * Generic Component: event fires once and the top-level generic
    Component is removed via the recursive Device.RemoveComponent.
  * Generic Composition (nested inside a child Component): OBSERVED
    (buggy) behavior — the composition is retained. Device.RemoveComposition
    (Device.cs:664) only removes from Device.Compositions (top-level);
    it does NOT recurse into child Components. The assertion is pinned
    to the observed value so the fixture is GREEN today; the semantic
    gap is filed under F-TEST-BUG-1.
  * Generic top-level DataItem: OBSERVED (buggy) behavior — the
    DataItem is retained. Device.RemoveDataItem (Device.cs:1017)
    OVERRIDES the base Component.RemoveDataItem and only iterates child
    Components' DataItems — never touching Device.DataItems. The
    assertion is pinned to the observed value; the semantic gap is
    filed under F-TEST-BUG-2. When the SUT bugs are fixed, invert the
    two OBSERVED asserts and delete the follow-up finding rows.

Verified GREEN on bluefin against the PR head (a2eebe0):
  Passed! - Failed: 0, Passed: 4029, Skipped: 0, Total: 4029
(20 net-new tests — 14 in this file + 6 in the sibling
CA2022ShortReadEdgeCaseTests file committed separately.)
@ottobolyos
ottobolyos force-pushed the fix/release-pack-warnings-cleanup branch from 30c330d to c4e1431 Compare August 19, 2026 12:12
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 19, 2026
…ce.RemoveDataItem

Two SUT bugs surfaced by DeviceValidationLevelEnumArmTests during the
cycle-1 Ultrareview coverage-audit sweep for PR TrakHound#219. Both live in the
DeviceValidationLevel.Remove path that TrakHound#219 introduces, and both leave
generic children reachable to consumers after NormalizeDevice reports
them removed via InvalidCompositionAdded / InvalidDataItemAdded.

F-TEST-BUG-1 — Device.RemoveComposition(string) (Device.cs:664)

  Only removed from Device.Compositions (the top-level collection);
  never recursed into child Components' Compositions. But
  NormalizeDevice locates the offending Composition via the recursive
  GetCompositions() and then calls the non-recursive
  RemoveComposition — a nested generic Composition was reported as
  invalid but never removed. Fix: mirror the shape of the recursive
  Device.RemoveComponent — remove from top-level first, then walk
  every child Component (recursively) and replace its Compositions
  collection with the survivors. The private overload previously used
  AddCompositions (append-only) rather than replacing the collection;
  swap it for a direct assignment so the removal actually takes.

F-TEST-BUG-2 — Device.RemoveDataItem(string) (Device.cs:1017)

  OVERRODE Component.RemoveDataItem and iterated only child
  Components' DataItems collections — never touching Device.DataItems
  itself. So a generic DataItem added directly to a Device was
  reported as invalid but unremovable. Fix: prepend a top-level
  Device.DataItems removal pass before descending into child
  Components.

Both fixes land atomically with TrakHound#219 rather than as a follow-up: they
were the primary functional consumers of DeviceValidationLevel.Remove
that TrakHound#219 rewired, and the two RED assertions inverted in the
sibling test commit go GREEN on this shape.
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 19, 2026
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 19, 2026
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 19, 2026
Ultrareview cycle 1 coverage-audit finding F-TEST-001: PR TrakHound#219 commit
90daffc added the DeviceValidationLevel enum plus an
AgentConfiguration.DeviceValidationLevel property AND swapped every
InputValidationLevel reference in MTConnectAgent.NormalizeDevice
(MTConnectAgent.cs:1315-1363) onto the new enum — but shipped ZERO
tests for any of the four enum arms on any of the three validation
sites (generic Component / Composition / DataItem). That is a
12-cell (arm × site) FLOOR gap under CONVENTIONS §1.0d-trigies-novodecies
plus a TDD-ordering violation under §1.0d-trigies-octies (feat commit
with no preceding RED test).

Adds tests/MTConnect.NET-Common-Tests/Agents/DeviceValidationLevelEnumArmTests.cs
which pins:

- 4 arms × 3 sites = 12 (arm × site) branch contracts;
- The AgentConfiguration.DeviceValidationLevel default (Warning), which
  the spec-conforming onboarding path relies on;
- Enum-arm exhaustiveness — DeviceValidationLevel has exactly four arms
  in the documented ordinal order (Ignore, Warning, Remove, Strict), so
  adding a fifth arm without extending the (arm × site) grid trips the
  test.

Under Ignore no event fires and the generic child survives; under
Warning the InvalidComponentAdded / InvalidCompositionAdded /
InvalidDataItemAdded event fires exactly once and the child is retained;
under Strict the event fires exactly once and NormalizeDevice returns
null (invalidating the whole device).

Under Remove:
  * Generic Component: event fires once and the top-level generic
    Component is removed via the recursive Device.RemoveComponent.
  * Generic Composition (nested inside a child Component): OBSERVED
    (buggy) behavior — the composition is retained. Device.RemoveComposition
    (Device.cs:664) only removes from Device.Compositions (top-level);
    it does NOT recurse into child Components. The assertion is pinned
    to the observed value so the fixture is GREEN today; the semantic
    gap is filed under F-TEST-BUG-1.
  * Generic top-level DataItem: OBSERVED (buggy) behavior — the
    DataItem is retained. Device.RemoveDataItem (Device.cs:1017)
    OVERRIDES the base Component.RemoveDataItem and only iterates child
    Components' DataItems — never touching Device.DataItems. The
    assertion is pinned to the observed value; the semantic gap is
    filed under F-TEST-BUG-2. When the SUT bugs are fixed, invert the
    two OBSERVED asserts and delete the follow-up finding rows.

Verified GREEN on bluefin against the PR head (a2eebe0):
  Passed! - Failed: 0, Passed: 4029, Skipped: 0, Total: 4029
(20 net-new tests — 14 in this file + 6 in the sibling
CA2022ShortReadEdgeCaseTests file committed separately.)
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 19, 2026
…ce.RemoveDataItem

Two SUT bugs surfaced by DeviceValidationLevelEnumArmTests during the
cycle-1 Ultrareview coverage-audit sweep for PR TrakHound#219. Both live in the
DeviceValidationLevel.Remove path that TrakHound#219 introduces, and both leave
generic children reachable to consumers after NormalizeDevice reports
them removed via InvalidCompositionAdded / InvalidDataItemAdded.

F-TEST-BUG-1 — Device.RemoveComposition(string) (Device.cs:664)

  Only removed from Device.Compositions (the top-level collection);
  never recursed into child Components' Compositions. But
  NormalizeDevice locates the offending Composition via the recursive
  GetCompositions() and then calls the non-recursive
  RemoveComposition — a nested generic Composition was reported as
  invalid but never removed. Fix: mirror the shape of the recursive
  Device.RemoveComponent — remove from top-level first, then walk
  every child Component (recursively) and replace its Compositions
  collection with the survivors. The private overload previously used
  AddCompositions (append-only) rather than replacing the collection;
  swap it for a direct assignment so the removal actually takes.

F-TEST-BUG-2 — Device.RemoveDataItem(string) (Device.cs:1017)

  OVERRODE Component.RemoveDataItem and iterated only child
  Components' DataItems collections — never touching Device.DataItems
  itself. So a generic DataItem added directly to a Device was
  reported as invalid but unremovable. Fix: prepend a top-level
  Device.DataItems removal pass before descending into child
  Components.

Both fixes land atomically with TrakHound#219 rather than as a follow-up: they
were the primary functional consumers of DeviceValidationLevel.Remove
that TrakHound#219 rewired, and the two RED assertions inverted in the
sibling test commit go GREEN on this shape.
@ottobolyos
ottobolyos force-pushed the fix/release-pack-warnings-cleanup branch from 5f17cc5 to 4e9ea44 Compare August 19, 2026 22:05
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 19, 2026
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 20, 2026
Sibling of 8e5eed8 (ReadRequestBytes unify) — deletes the
NET9_0_OR_GREATER guard around ReadExactlyAsync in
LimitedBodyStream.DiscardAllAsync so the drain path uses the same
accumulator loop on every TFM. ReadExactlyAsync into a fixed 8 KB
buffer throws EndOfStreamException on any body smaller than 8 KB
and on the final iteration of larger drains — that exception then
propagates through the outer HttpServer catch, kills keep-alive,
and 500s the client. The uniform ReadAsync loop treats a 0-byte
read as premature EOF and signals the caller cleanly.

Extracted from PR TrakHound#219 cycle-2 F-CR-201 during the 2026-08-20
clean-split of the empty-Result / multi-TFM / warnings-cleanup
three-way commit contamination — the other content of the
original mixed commit belongs to PR TrakHound#217, so only the
LimitedBodyStream.cs change ships here.
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 20, 2026
Adds a testing/workflows.md section describing the release-pack
job introduced by 1e96847 (the CI matrix gate) — trigger scope,
exit contract, and the local dotnet pack repro command. The
docs make it explicit that the release-pack matrix runs the full
net461 → net9.0 sweep on every push and non-draft PR, catching
the class of Release-only CS/NU/CA/SYSLIB diagnostics the Debug
matrix does not exercise.

Extracted from PR TrakHound#219 cycle-1 F-DOC-005 during the 2026-08-20
clean-split — the other content of the mixed source commit
belongs to PR TrakHound#217 (DeviceValidationLevel docs), so only the
workflows.md addition ships here.
@ottobolyos
ottobolyos force-pushed the fix/release-pack-warnings-cleanup branch from 4e9ea44 to 2188d5d Compare August 20, 2026 12:20
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 20, 2026
Add a HashSet<string> visited-Id cycle guard threaded through every
recursive walk under Device.RemoveComposition, Device.RemoveDataItem,
Component.RemoveComposition and Component.RemoveDataItem, with a
belt-and-braces depth ceiling at 1024. A cyclic Component graph
(A.Components ∋ B, B.Components ∋ A) previously walked the recursion
until the process stack exhausted; the guard terminates the walk
after every node's Id is visited once.

Rewrite Component.RemoveComposition to recurse across nested child
Components (the sibling site the audit brief M2 called out as still
non-recursive after PR TrakHound#219's Device.cs recursion fix) and rewrite
Component.RemoveDataItem to walk children inline instead of routing
through the unguarded Component.GetComponents() flatten, so the cycle
guard applies uniformly to the DataItem path too.

Delete the zero-caller private RemoveComposition(IComponent, string)
helper on Component (the append-duplicates AddCompositions
anti-pattern from before PR TrakHound#219).

Test: DeviceRemoveRecursionTests gains four cycle-guard fixtures
(RemoveComposition_terminates_on_cyclic_Component_graph,
RemoveDataItem_terminates_on_cyclic_Component_graph,
Component_RemoveComposition_reaches_nested_and_terminates_on_cycle,
Component_RemoveDataItem_terminates_on_cyclic_Component_graph)
pinning the observable termination guarantee — a regression to
unguarded recursion fails as a StackOverflowException.

Refs: dime Ultrareview cycle-1 findings H1 (bug-detector + security
A04) and M2 (bug-class atomicity per CONVENTIONS §1.0d-trigies-bis).
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 21, 2026
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 21, 2026
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 21, 2026
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 21, 2026
@ottobolyos

Copy link
Copy Markdown
Contributor Author

@PatrickRitchie — five additional dime cycles landed on top of the cycle-1-3 head. New head d793ae8b; 21 commits total on the queue head, all signed. The PR body now carries the cycle 1-8 ledger.

Substantive additions since the earlier ready-flip attempt:

  • CancellationToken threading across the whole vendored Ceen HTTP body-I/O surface. The dime cycle 4 improvement pass surfaced that MTConnectPostResponseHandler.ReadRequestBytes silently ignored caller-driven cancellation. Cycles 5, 6, and 7 walked the bug class through every sibling site: SimpleProxyHandler request + response copy, RestApiHelper.Post / PutDetail, and finally the response-write path via a new IHttpResponse.WriteAllAsync(Stream, CancellationToken, string) overload that flows through MTConnectHttpResponseHandler.WriteResponseWriteToStream → the four gzip / br / deflate / plain WriteAllAsync call sites. Every site uses the universal 3-arg CopyToAsync(Stream, int bufferSize, CancellationToken) overload so net461 → net10.0 all honour the token uniformly; the legacy tokenless overloads remain as source-compat forwarders.
  • ReadRequestBytes catch split. OperationCanceledException now propagates rather than being swallowed into the null-return contract (so client aborts reach telemetry instead of masking as 404s). The general catch still preserves the "malformed asset POST returns null" contract, plus a CR/LF-sanitised System.Diagnostics.Trace.WriteLine breadcrumb naming the swallowed exception type (BCL-only, costs nothing when no listener is attached, OWASP A09 log-format-injection defence).
  • Release-pack CI regex widened. From the enumerated (CS|CA|NU|SYSLIB|MSB) prefixes to the shape-based \b(error|warning)[[:space:]]+[A-Za-z]+[0-9]+, so IL / NETSDK / StyleCop / mixed-case analyzer families (xUnit1004, nunit1001) also surface in failure summaries. Truncation-hint breadcrumb added above the fenced block naming the unique + total diagnostic counts.
  • Two new RED-first tests on the pre-cancelled + cancelled-mid-drip boundaries of ReadRequestBytes; the full 4,096-test MTConnect.NET-Common-Tests suite stays green on bluefin (net8.0). Multi-TFM Release-pack build (net461 → net10.0) also green (0 errors, 9 pre-existing net461 nuget-compat warnings).

Non-atomic follow-ups filed against TrakHound/MTConnect.NET:

  • #254 — ArrayPool for the 2 MB LOH buffer in the asset-POST accumulator (volume-gated).
  • #255 — consolidate OneByteAtATimeStream into ScriptedStream across CA2022 fixtures.
  • #256 — extract shared Ca2022Reflection.LoadHandlerType() helper + RestApiHelper.ReadJsonBodyAsync helper.
  • #257 — multipart form parser CT threading (HttpRequest.cs:437).
  • #258 — aggregate residual vendored Ceen CT-threading (streaming endpoints, FileHandler / FileMirrorHandler read loops, HttpResponse chunked-encoding internal helpers).

Cycle 8 verification returned zero unfixed findings across code-review + security-audit + improvement passes — every remaining tokenless site in the vendored tree is either covered by an existing tracking issue, is <Compile Remove>d as dead vendored code, or has no MTConnect production caller.

ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 21, 2026
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 21, 2026
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 21, 2026
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 21, 2026
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 21, 2026
@PatrickRitchie PatrickRitchie moved this from Reviewing to Ready to Merge in MTConnect.NET-Development Aug 21, 2026
@PatrickRitchie
PatrickRitchie merged commit 7626218 into TrakHound:master Aug 21, 2026
18 checks passed
@github-project-automation github-project-automation Bot moved this from Ready to Merge to Done in MTConnect.NET-Development Aug 21, 2026
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 21, 2026
…ce.RemoveDataItem

Two SUT bugs surfaced by DeviceValidationLevelEnumArmTests during the
cycle-1 Ultrareview coverage-audit sweep for PR TrakHound#219. Both live in the
DeviceValidationLevel.Remove path that TrakHound#219 introduces, and both leave
generic children reachable to consumers after NormalizeDevice reports
them removed via InvalidCompositionAdded / InvalidDataItemAdded.

F-TEST-BUG-1 — Device.RemoveComposition(string) (Device.cs:664)

  Only removed from Device.Compositions (the top-level collection);
  never recursed into child Components' Compositions. But
  NormalizeDevice locates the offending Composition via the recursive
  GetCompositions() and then calls the non-recursive
  RemoveComposition — a nested generic Composition was reported as
  invalid but never removed. Fix: mirror the shape of the recursive
  Device.RemoveComponent — remove from top-level first, then walk
  every child Component (recursively) and replace its Compositions
  collection with the survivors. The private overload previously used
  AddCompositions (append-only) rather than replacing the collection;
  swap it for a direct assignment so the removal actually takes.

F-TEST-BUG-2 — Device.RemoveDataItem(string) (Device.cs:1017)

  OVERRODE Component.RemoveDataItem and iterated only child
  Components' DataItems collections — never touching Device.DataItems
  itself. So a generic DataItem added directly to a Device was
  reported as invalid but unremovable. Fix: prepend a top-level
  Device.DataItems removal pass before descending into child
  Components.

Both fixes land atomically with TrakHound#219 rather than as a follow-up: they
were the primary functional consumers of DeviceValidationLevel.Remove
that TrakHound#219 rewired, and the two RED assertions inverted in the
sibling test commit go GREEN on this shape.
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 21, 2026
Ultrareview cycle 1 coverage-audit finding F-TEST-001: PR TrakHound#219 commit
90daffc added the DeviceValidationLevel enum plus an
AgentConfiguration.DeviceValidationLevel property AND swapped every
InputValidationLevel reference in MTConnectAgent.NormalizeDevice
(MTConnectAgent.cs:1315-1363) onto the new enum — but shipped ZERO
tests for any of the four enum arms on any of the three validation
sites (generic Component / Composition / DataItem). That is a
12-cell (arm × site) FLOOR gap under CONVENTIONS §1.0d-trigies-novodecies
plus a TDD-ordering violation under §1.0d-trigies-octies (feat commit
with no preceding RED test).

Adds tests/MTConnect.NET-Common-Tests/Agents/DeviceValidationLevelEnumArmTests.cs
which pins:

- 4 arms × 3 sites = 12 (arm × site) branch contracts;
- The AgentConfiguration.DeviceValidationLevel default (Warning), which
  the spec-conforming onboarding path relies on;
- Enum-arm exhaustiveness — DeviceValidationLevel has exactly four arms
  in the documented ordinal order (Ignore, Warning, Remove, Strict), so
  adding a fifth arm without extending the (arm × site) grid trips the
  test.

Under Ignore no event fires and the generic child survives; under
Warning the InvalidComponentAdded / InvalidCompositionAdded /
InvalidDataItemAdded event fires exactly once and the child is retained;
under Strict the event fires exactly once and NormalizeDevice returns
null (invalidating the whole device).

Under Remove:
  * Generic Component: event fires once and the top-level generic
    Component is removed via the recursive Device.RemoveComponent.
  * Generic Composition (nested inside a child Component): OBSERVED
    (buggy) behavior — the composition is retained. Device.RemoveComposition
    (Device.cs:664) only removes from Device.Compositions (top-level);
    it does NOT recurse into child Components. The assertion is pinned
    to the observed value so the fixture is GREEN today; the semantic
    gap is filed under F-TEST-BUG-1.
  * Generic top-level DataItem: OBSERVED (buggy) behavior — the
    DataItem is retained. Device.RemoveDataItem (Device.cs:1017)
    OVERRIDES the base Component.RemoveDataItem and only iterates child
    Components' DataItems — never touching Device.DataItems. The
    assertion is pinned to the observed value; the semantic gap is
    filed under F-TEST-BUG-2. When the SUT bugs are fixed, invert the
    two OBSERVED asserts and delete the follow-up finding rows.

Verified GREEN on bluefin against the PR head (a2eebe0):
  Passed! - Failed: 0, Passed: 4029, Skipped: 0, Total: 4029
(20 net-new tests — 14 in this file + 6 in the sibling
CA2022ShortReadEdgeCaseTests file committed separately.)
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 21, 2026
Add a HashSet<string> visited-Id cycle guard threaded through every
recursive walk under Device.RemoveComposition, Device.RemoveDataItem,
Component.RemoveComposition and Component.RemoveDataItem, with a
belt-and-braces depth ceiling at 1024. A cyclic Component graph
(A.Components ∋ B, B.Components ∋ A) previously walked the recursion
until the process stack exhausted; the guard terminates the walk
after every node's Id is visited once.

Rewrite Component.RemoveComposition to recurse across nested child
Components (the sibling site the audit brief M2 called out as still
non-recursive after PR TrakHound#219's Device.cs recursion fix) and rewrite
Component.RemoveDataItem to walk children inline instead of routing
through the unguarded Component.GetComponents() flatten, so the cycle
guard applies uniformly to the DataItem path too.

Delete the zero-caller private RemoveComposition(IComponent, string)
helper on Component (the append-duplicates AddCompositions
anti-pattern from before PR TrakHound#219).

Test: DeviceRemoveRecursionTests gains four cycle-guard fixtures
(RemoveComposition_terminates_on_cyclic_Component_graph,
RemoveDataItem_terminates_on_cyclic_Component_graph,
Component_RemoveComposition_reaches_nested_and_terminates_on_cycle,
Component_RemoveDataItem_terminates_on_cyclic_Component_graph)
pinning the observable termination guarantee — a regression to
unguarded recursion fails as a StackOverflowException.

Refs: dime Ultrareview cycle-1 findings H1 (bug-detector + security
A04) and M2 (bug-class atomicity per CONVENTIONS §1.0d-trigies-bis).
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 21, 2026
Two SUT bugs surfaced by DeviceValidationLevelEnumArmTests during the
cycle-1 Ultrareview coverage-audit sweep for PR TrakHound#219. Both live in the
DeviceValidationLevel.Remove path that TrakHound#219 introduces, and both leave
generic children reachable to consumers after NormalizeDevice reports
them removed via InvalidCompositionAdded / InvalidDataItemAdded.

F-TEST-BUG-1 — Device.RemoveComposition(string) (Device.cs:664)

  Only removed from Device.Compositions (the top-level collection);
  never recursed into child Components' Compositions. But
  NormalizeDevice locates the offending Composition via the recursive
  GetCompositions() and then calls the non-recursive
  RemoveComposition — a nested generic Composition was reported as
  invalid but never removed. Fix: mirror the shape of the recursive
  Device.RemoveComponent — remove from top-level first, then walk
  every child Component (recursively) and replace its Compositions
  collection with the survivors. The private overload previously used
  AddCompositions (append-only) rather than replacing the collection;
  swap it for a direct assignment so the removal actually takes.

F-TEST-BUG-2 — Device.RemoveDataItem(string) (Device.cs:1017)

  OVERRODE Component.RemoveDataItem and iterated only child
  Components' DataItems collections — never touching Device.DataItems
  itself. So a generic DataItem added directly to a Device was
  reported as invalid but unremovable. Fix: prepend a top-level
  Device.DataItems removal pass before descending into child
  Components.

Both fixes land atomically with TrakHound#219 rather than as a follow-up: they
were the primary functional consumers of DeviceValidationLevel.Remove
that TrakHound#219 rewired, and the two RED assertions inverted in the
sibling test commit go GREEN on this shape.
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 21, 2026
Ultrareview cycle 1 coverage-audit finding F-TEST-001: PR TrakHound#219 commit
90daffc added the DeviceValidationLevel enum plus an
AgentConfiguration.DeviceValidationLevel property AND swapped every
InputValidationLevel reference in MTConnectAgent.NormalizeDevice
(MTConnectAgent.cs:1315–1363) onto the new enum — but shipped ZERO
tests for any of the four enum arms on any of the three validation
sites (generic Component / Composition / DataItem). That is a
12-cell (arm × site) FLOOR gap under CONVENTIONS §1.0d-trigies-novodecies
plus a TDD-ordering violation under §1.0d-trigies-octies (feat commit
with no preceding RED test).

Adds tests/MTConnect.NET-Common-Tests/Agents/DeviceValidationLevelEnumArmTests.cs
which pins:

- 4 arms × 3 sites = 12 (arm × site) branch contracts;
- The AgentConfiguration.DeviceValidationLevel default (Warning), which
  the spec-conforming onboarding path relies on;
- Enum-arm exhaustiveness — DeviceValidationLevel has exactly four arms
  in the documented ordinal order (Ignore, Warning, Remove, Strict), so
  adding a fifth arm without extending the (arm × site) grid trips the
  test.

Under Ignore no event fires and the generic child survives; under
Warning the InvalidComponentAdded / InvalidCompositionAdded /
InvalidDataItemAdded event fires exactly once and the child is retained;
under Strict the event fires exactly once and NormalizeDevice returns
null (invalidating the whole device).

Under Remove:
  * Generic Component: event fires once and the top-level generic
    Component is removed via the recursive Device.RemoveComponent.
  * Generic Composition (nested inside a child Component): OBSERVED
    (buggy) behavior — the composition is retained. Device.RemoveComposition
    (Device.cs:664) only removes from Device.Compositions (top-level);
    it does NOT recurse into child Components. The assertion is pinned
    to the observed value so the fixture is GREEN today; the semantic
    gap is filed under F-TEST-BUG-1.
  * Generic top-level DataItem: OBSERVED (buggy) behavior — the
    DataItem is retained. Device.RemoveDataItem (Device.cs:1017)
    OVERRIDES the base Component.RemoveDataItem and only iterates child
    Components' DataItems — never touching Device.DataItems. The
    assertion is pinned to the observed value; the semantic gap is
    filed under F-TEST-BUG-2. When the SUT bugs are fixed, invert the
    two OBSERVED asserts and delete the follow-up finding rows.

Verified GREEN on bluefin against the PR head (a2eebe0):
  Passed! - Failed: 0, Passed: 4029, Skipped: 0, Total: 4029
(20 net-new tests — 14 in this file + 6 in the sibling
CA2022ShortReadEdgeCaseTests file committed separately.)
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 21, 2026
Add a HashSet<string> visited-Id cycle guard threaded through every
recursive walk under Device.RemoveComposition, Device.RemoveDataItem,
Component.RemoveComposition and Component.RemoveDataItem, with a
belt-and-braces depth ceiling at 1024. A cyclic Component graph
(A.Components ∋ B, B.Components ∋ A) previously walked the recursion
until the process stack exhausted; the guard terminates the walk
after every node's Id is visited once.

Rewrite Component.RemoveComposition to recurse across nested child
Components (the sibling site the audit brief M2 called out as still
non-recursive after PR TrakHound#219's Device.cs recursion fix) and rewrite
Component.RemoveDataItem to walk children inline instead of routing
through the unguarded Component.GetComponents() flatten, so the cycle
guard applies uniformly to the DataItem path too.

Delete the zero-caller private RemoveComposition(IComponent, string)
helper on Component (the append-duplicates AddCompositions
anti-pattern from before PR TrakHound#219).

Test: DeviceRemoveRecursionTests gains four cycle-guard fixtures
(RemoveComposition_terminates_on_cyclic_Component_graph,
RemoveDataItem_terminates_on_cyclic_Component_graph,
Component_RemoveComposition_reaches_nested_and_terminates_on_cycle,
Component_RemoveDataItem_terminates_on_cyclic_Component_graph)
pinning the observable termination guarantee — a regression to
unguarded recursion fails as a StackOverflowException.

Refs: dime Ultrareview cycle-1 findings H1 (bug-detector + security
A04) and M2 (bug-class atomicity per CONVENTIONS §1.0d-trigies-bis).
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 21, 2026
…ce.RemoveDataItem

Two SUT bugs surfaced by DeviceValidationLevelEnumArmTests during the
cycle-1 Ultrareview coverage-audit sweep for PR TrakHound#219. Both live in the
DeviceValidationLevel.Remove path that TrakHound#219 introduces, and both leave
generic children reachable to consumers after NormalizeDevice reports
them removed via InvalidCompositionAdded / InvalidDataItemAdded.

F-TEST-BUG-1 — Device.RemoveComposition(string) (Device.cs:664)

  Only removed from Device.Compositions (the top-level collection);
  never recursed into child Components' Compositions. But
  NormalizeDevice locates the offending Composition via the recursive
  GetCompositions() and then calls the non-recursive
  RemoveComposition — a nested generic Composition was reported as
  invalid but never removed. Fix: mirror the shape of the recursive
  Device.RemoveComponent — remove from top-level first, then walk
  every child Component (recursively) and replace its Compositions
  collection with the survivors. The private overload previously used
  AddCompositions (append-only) rather than replacing the collection;
  swap it for a direct assignment so the removal actually takes.

F-TEST-BUG-2 — Device.RemoveDataItem(string) (Device.cs:1017)

  OVERRODE Component.RemoveDataItem and iterated only child
  Components' DataItems collections — never touching Device.DataItems
  itself. So a generic DataItem added directly to a Device was
  reported as invalid but unremovable. Fix: prepend a top-level
  Device.DataItems removal pass before descending into child
  Components.

Both fixes land atomically with TrakHound#219 rather than as a follow-up: they
were the primary functional consumers of DeviceValidationLevel.Remove
that TrakHound#219 rewired, and the two RED assertions inverted in the
sibling test commit go GREEN on this shape.
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 21, 2026
Ultrareview cycle 1 coverage-audit finding F-TEST-001: PR TrakHound#219 commit
90daffc added the DeviceValidationLevel enum plus an
AgentConfiguration.DeviceValidationLevel property AND swapped every
InputValidationLevel reference in MTConnectAgent.NormalizeDevice
(MTConnectAgent.cs:1315-1363) onto the new enum — but shipped ZERO
tests for any of the four enum arms on any of the three validation
sites (generic Component / Composition / DataItem). That is a
12-cell (arm × site) FLOOR gap under CONVENTIONS §1.0d-trigies-novodecies
plus a TDD-ordering violation under §1.0d-trigies-octies (feat commit
with no preceding RED test).

Adds tests/MTConnect.NET-Common-Tests/Agents/DeviceValidationLevelEnumArmTests.cs
which pins:

- 4 arms × 3 sites = 12 (arm × site) branch contracts;
- The AgentConfiguration.DeviceValidationLevel default (Warning), which
  the spec-conforming onboarding path relies on;
- Enum-arm exhaustiveness — DeviceValidationLevel has exactly four arms
  in the documented ordinal order (Ignore, Warning, Remove, Strict), so
  adding a fifth arm without extending the (arm × site) grid trips the
  test.

Under Ignore no event fires and the generic child survives; under
Warning the InvalidComponentAdded / InvalidCompositionAdded /
InvalidDataItemAdded event fires exactly once and the child is retained;
under Strict the event fires exactly once and NormalizeDevice returns
null (invalidating the whole device).

Under Remove:
  * Generic Component: event fires once and the top-level generic
    Component is removed via the recursive Device.RemoveComponent.
  * Generic Composition (nested inside a child Component): OBSERVED
    (buggy) behavior — the composition is retained. Device.RemoveComposition
    (Device.cs:664) only removes from Device.Compositions (top-level);
    it does NOT recurse into child Components. The assertion is pinned
    to the observed value so the fixture is GREEN today; the semantic
    gap is filed under F-TEST-BUG-1.
  * Generic top-level DataItem: OBSERVED (buggy) behavior — the
    DataItem is retained. Device.RemoveDataItem (Device.cs:1017)
    OVERRIDES the base Component.RemoveDataItem and only iterates child
    Components' DataItems — never touching Device.DataItems. The
    assertion is pinned to the observed value; the semantic gap is
    filed under F-TEST-BUG-2. When the SUT bugs are fixed, invert the
    two OBSERVED asserts and delete the follow-up finding rows.

Verified GREEN on bluefin against the PR head (a2eebe0):
  Passed! - Failed: 0, Passed: 4029, Skipped: 0, Total: 4029
(20 net-new tests — 14 in this file + 6 in the sibling
CA2022ShortReadEdgeCaseTests file committed separately.)
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 21, 2026
Two SUT bugs surfaced by DeviceValidationLevelEnumArmTests during the
cycle-1 Ultrareview coverage-audit sweep for PR TrakHound#219. Both live in the
DeviceValidationLevel.Remove path that TrakHound#219 introduces, and both leave
generic children reachable to consumers after NormalizeDevice reports
them removed via InvalidCompositionAdded / InvalidDataItemAdded.

F-TEST-BUG-1 — Device.RemoveComposition(string) (Device.cs:664)

  Only removed from Device.Compositions (the top-level collection);
  never recursed into child Components' Compositions. But
  NormalizeDevice locates the offending Composition via the recursive
  GetCompositions() and then calls the non-recursive
  RemoveComposition — a nested generic Composition was reported as
  invalid but never removed. Fix: mirror the shape of the recursive
  Device.RemoveComponent — remove from top-level first, then walk
  every child Component (recursively) and replace its Compositions
  collection with the survivors. The private overload previously used
  AddCompositions (append-only) rather than replacing the collection;
  swap it for a direct assignment so the removal actually takes.

F-TEST-BUG-2 — Device.RemoveDataItem(string) (Device.cs:1017)

  OVERRODE Component.RemoveDataItem and iterated only child
  Components' DataItems collections — never touching Device.DataItems
  itself. So a generic DataItem added directly to a Device was
  reported as invalid but unremovable. Fix: prepend a top-level
  Device.DataItems removal pass before descending into child
  Components.

Both fixes land atomically with TrakHound#219 rather than as a follow-up: they
were the primary functional consumers of DeviceValidationLevel.Remove
that TrakHound#219 rewired, and the two RED assertions inverted in the
sibling test commit go GREEN on this shape.
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 21, 2026
Ultrareview cycle 1 coverage-audit finding F-TEST-001: PR TrakHound#219 commit
90daffc added the DeviceValidationLevel enum plus an
AgentConfiguration.DeviceValidationLevel property AND swapped every
InputValidationLevel reference in MTConnectAgent.NormalizeDevice
(MTConnectAgent.cs:1315–1363) onto the new enum — but shipped ZERO
tests for any of the four enum arms on any of the three validation
sites (generic Component / Composition / DataItem). That is a
12-cell (arm × site) FLOOR gap under CONVENTIONS §1.0d-trigies-novodecies
plus a TDD-ordering violation under §1.0d-trigies-octies (feat commit
with no preceding RED test).

Adds tests/MTConnect.NET-Common-Tests/Agents/DeviceValidationLevelEnumArmTests.cs
which pins:

- 4 arms × 3 sites = 12 (arm × site) branch contracts;
- The AgentConfiguration.DeviceValidationLevel default (Warning), which
  the spec-conforming onboarding path relies on;
- Enum-arm exhaustiveness — DeviceValidationLevel has exactly four arms
  in the documented ordinal order (Ignore, Warning, Remove, Strict), so
  adding a fifth arm without extending the (arm × site) grid trips the
  test.

Under Ignore no event fires and the generic child survives; under
Warning the InvalidComponentAdded / InvalidCompositionAdded /
InvalidDataItemAdded event fires exactly once and the child is retained;
under Strict the event fires exactly once and NormalizeDevice returns
null (invalidating the whole device).

Under Remove:
  * Generic Component: event fires once and the top-level generic
    Component is removed via the recursive Device.RemoveComponent.
  * Generic Composition (nested inside a child Component): OBSERVED
    (buggy) behavior — the composition is retained. Device.RemoveComposition
    (Device.cs:664) only removes from Device.Compositions (top-level);
    it does NOT recurse into child Components. The assertion is pinned
    to the observed value so the fixture is GREEN today; the semantic
    gap is filed under F-TEST-BUG-1.
  * Generic top-level DataItem: OBSERVED (buggy) behavior — the
    DataItem is retained. Device.RemoveDataItem (Device.cs:1017)
    OVERRIDES the base Component.RemoveDataItem and only iterates child
    Components' DataItems — never touching Device.DataItems. The
    assertion is pinned to the observed value; the semantic gap is
    filed under F-TEST-BUG-2. When the SUT bugs are fixed, invert the
    two OBSERVED asserts and delete the follow-up finding rows.

Verified GREEN on bluefin against the PR head (a2eebe0):
  Passed! - Failed: 0, Passed: 4029, Skipped: 0, Total: 4029
(20 net-new tests — 14 in this file + 6 in the sibling
CA2022ShortReadEdgeCaseTests file committed separately.)
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 21, 2026
Add a HashSet<string> visited-Id cycle guard threaded through every
recursive walk under Device.RemoveComposition, Device.RemoveDataItem,
Component.RemoveComposition and Component.RemoveDataItem, with a
belt-and-braces depth ceiling at 1024. A cyclic Component graph
(A.Components ∋ B, B.Components ∋ A) previously walked the recursion
until the process stack exhausted; the guard terminates the walk
after every node's Id is visited once.

Rewrite Component.RemoveComposition to recurse across nested child
Components (the sibling site the audit brief M2 called out as still
non-recursive after PR TrakHound#219's Device.cs recursion fix) and rewrite
Component.RemoveDataItem to walk children inline instead of routing
through the unguarded Component.GetComponents() flatten, so the cycle
guard applies uniformly to the DataItem path too.

Delete the zero-caller private RemoveComposition(IComponent, string)
helper on Component (the append-duplicates AddCompositions
anti-pattern from before PR TrakHound#219).

Test: DeviceRemoveRecursionTests gains four cycle-guard fixtures
(RemoveComposition_terminates_on_cyclic_Component_graph,
RemoveDataItem_terminates_on_cyclic_Component_graph,
Component_RemoveComposition_reaches_nested_and_terminates_on_cycle,
Component_RemoveDataItem_terminates_on_cyclic_Component_graph)
pinning the observable termination guarantee — a regression to
unguarded recursion fails as a StackOverflowException.

Refs: dime Ultrareview cycle-1 findings H1 (bug-detector + security
A04) and M2 (bug-class atomicity per CONVENTIONS §1.0d-trigies-bis).
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 21, 2026
NUnit 4 removed the AsyncTestDelegate overload for Assert.That in favour of
Func<Task> — see 'Use Func<Task> instead of AsyncTestDelegate' obsoletion in
nunit.framework.dll 4.6.1. CA2022ShortReadEdgeCaseTests.cs (arrived on
master via merged TrakHound#219) has two Assert.That(async () => ..., Throws.*) sites
that trigger CS0121 overload ambiguity once NUnit 4 is active. Wrapping the
async lambdas with (Func<Task>) disambiguates unambiguously.

This closes the last of the 9-file Bug A cast-fix pass surfaced on the
prior integration/up-to-pr-249 build (158 errors, 112 CS0121). The other
6 branches carry (Action) casts on sync sites in their own tests.
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 22, 2026
NUnit 4 removed the AsyncTestDelegate overload for Assert.That in favour of
Func<Task> — see 'Use Func<Task> instead of AsyncTestDelegate' obsoletion in
nunit.framework.dll 4.6.1. CA2022ShortReadEdgeCaseTests.cs (arrived on
master via merged TrakHound#219) has two Assert.That(async () => ..., Throws.*) sites
that trigger CS0121 overload ambiguity once NUnit 4 is active. Wrapping the
async lambdas with (Func<Task>) disambiguates unambiguously.

This closes the last of the 9-file Bug A cast-fix pass surfaced on the
prior integration/up-to-pr-249 build (158 errors, 112 CS0121). The other
6 branches carry (Action) casts on sync sites in their own tests.
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 22, 2026
Two SUT bugs surfaced by DeviceValidationLevelEnumArmTests during the
cycle-1 Ultrareview coverage-audit sweep for PR TrakHound#219. Both live in the
DeviceValidationLevel.Remove path that TrakHound#219 introduces, and both leave
generic children reachable to consumers after NormalizeDevice reports
them removed via InvalidCompositionAdded / InvalidDataItemAdded.

F-TEST-BUG-1 — Device.RemoveComposition(string) (Device.cs:664)

  Only removed from Device.Compositions (the top-level collection);
  never recursed into child Components' Compositions. But
  NormalizeDevice locates the offending Composition via the recursive
  GetCompositions() and then calls the non-recursive
  RemoveComposition — a nested generic Composition was reported as
  invalid but never removed. Fix: mirror the shape of the recursive
  Device.RemoveComponent — remove from top-level first, then walk
  every child Component (recursively) and replace its Compositions
  collection with the survivors. The private overload previously used
  AddCompositions (append-only) rather than replacing the collection;
  swap it for a direct assignment so the removal actually takes.

F-TEST-BUG-2 — Device.RemoveDataItem(string) (Device.cs:1017)

  OVERRODE Component.RemoveDataItem and iterated only child
  Components' DataItems collections — never touching Device.DataItems
  itself. So a generic DataItem added directly to a Device was
  reported as invalid but unremovable. Fix: prepend a top-level
  Device.DataItems removal pass before descending into child
  Components.

Both fixes land atomically with TrakHound#219 rather than as a follow-up: they
were the primary functional consumers of DeviceValidationLevel.Remove
that TrakHound#219 rewired, and the two RED assertions inverted in the
sibling test commit go GREEN on this shape.
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 22, 2026
Ultrareview cycle 1 coverage-audit finding F-TEST-001: PR TrakHound#219 commit
90daffc added the DeviceValidationLevel enum plus an
AgentConfiguration.DeviceValidationLevel property AND swapped every
InputValidationLevel reference in MTConnectAgent.NormalizeDevice
(MTConnectAgent.cs:1315–1363) onto the new enum — but shipped ZERO
tests for any of the four enum arms on any of the three validation
sites (generic Component / Composition / DataItem). That is a
12-cell (arm × site) FLOOR gap under CONVENTIONS §1.0d-trigies-novodecies
plus a TDD-ordering violation under §1.0d-trigies-octies (feat commit
with no preceding RED test).

Adds tests/MTConnect.NET-Common-Tests/Agents/DeviceValidationLevelEnumArmTests.cs
which pins:

- 4 arms × 3 sites = 12 (arm × site) branch contracts;
- The AgentConfiguration.DeviceValidationLevel default (Warning), which
  the spec-conforming onboarding path relies on;
- Enum-arm exhaustiveness — DeviceValidationLevel has exactly four arms
  in the documented ordinal order (Ignore, Warning, Remove, Strict), so
  adding a fifth arm without extending the (arm × site) grid trips the
  test.

Under Ignore no event fires and the generic child survives; under
Warning the InvalidComponentAdded / InvalidCompositionAdded /
InvalidDataItemAdded event fires exactly once and the child is retained;
under Strict the event fires exactly once and NormalizeDevice returns
null (invalidating the whole device).

Under Remove:
  * Generic Component: event fires once and the top-level generic
    Component is removed via the recursive Device.RemoveComponent.
  * Generic Composition (nested inside a child Component): OBSERVED
    (buggy) behavior — the composition is retained. Device.RemoveComposition
    (Device.cs:664) only removes from Device.Compositions (top-level);
    it does NOT recurse into child Components. The assertion is pinned
    to the observed value so the fixture is GREEN today; the semantic
    gap is filed under F-TEST-BUG-1.
  * Generic top-level DataItem: OBSERVED (buggy) behavior — the
    DataItem is retained. Device.RemoveDataItem (Device.cs:1017)
    OVERRIDES the base Component.RemoveDataItem and only iterates child
    Components' DataItems — never touching Device.DataItems. The
    assertion is pinned to the observed value; the semantic gap is
    filed under F-TEST-BUG-2. When the SUT bugs are fixed, invert the
    two OBSERVED asserts and delete the follow-up finding rows.

Verified GREEN on bluefin against the PR head (a2eebe0):
  Passed! - Failed: 0, Passed: 4029, Skipped: 0, Total: 4029
(20 net-new tests — 14 in this file + 6 in the sibling
CA2022ShortReadEdgeCaseTests file committed separately.)
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 22, 2026
Add a HashSet<string> visited-Id cycle guard threaded through every
recursive walk under Device.RemoveComposition, Device.RemoveDataItem,
Component.RemoveComposition and Component.RemoveDataItem, with a
belt-and-braces depth ceiling at 1024. A cyclic Component graph
(A.Components ∋ B, B.Components ∋ A) previously walked the recursion
until the process stack exhausted; the guard terminates the walk
after every node's Id is visited once.

Rewrite Component.RemoveComposition to recurse across nested child
Components (the sibling site the audit brief M2 called out as still
non-recursive after PR TrakHound#219's Device.cs recursion fix) and rewrite
Component.RemoveDataItem to walk children inline instead of routing
through the unguarded Component.GetComponents() flatten, so the cycle
guard applies uniformly to the DataItem path too.

Delete the zero-caller private RemoveComposition(IComponent, string)
helper on Component (the append-duplicates AddCompositions
anti-pattern from before PR TrakHound#219).

Test: DeviceRemoveRecursionTests gains four cycle-guard fixtures
(RemoveComposition_terminates_on_cyclic_Component_graph,
RemoveDataItem_terminates_on_cyclic_Component_graph,
Component_RemoveComposition_reaches_nested_and_terminates_on_cycle,
Component_RemoveDataItem_terminates_on_cyclic_Component_graph)
pinning the observable termination guarantee — a regression to
unguarded recursion fails as a StackOverflowException.

Refs: dime Ultrareview cycle-1 findings H1 (bug-detector + security
A04) and M2 (bug-class atomicity per CONVENTIONS §1.0d-trigies-bis).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

2 participants