GithubStar/UA-.NETStandard
1
0
Fork 0
mirror of https://github.com/OPCFoundation/UA-.NETStandard.git synced 2026-10-18 14:43:08 +08:00
Code Issues Packages Projects Releases Wiki Activity
2,810 Commits 101 Branches 105 Tags
master
Commit Graph

8 Commits

Author SHA1 Message Date
romanett 9b034f8291 Enable nullable reference types centrally; migrate all unset and annotations-only projects (#4700) 
# Description

Nullable reference types are now enabled once, in `common.props`, and
every project in the repository builds warning-clean with them.

**Configuration**
- `<Nullable>enable</Nullable>` lives in `common.props`. All per-project
`<Nullable>` settings are removed: 178 project files (enable,
annotations, and the `disable` in the three Aot test projects).
- 419 per-file `#nullable enable`/`restore` directives and the 4
`#nullable disable` directives are removed.
- Kept on purpose:
- The 5 `<auto-generated>` schema files (`UANodeSet.cs`, `UA Type
Dictionary.cs`, `OPCBinarySchema.cs`, `SecuredApplication.cs`,
`PInvokeHelper.cs`) keep `#nullable enable`. The compiler ignores the
project setting in generated code, so the directive is required there.
- `tests/Opc.Ua.Interop.LegacyPeer` keeps `disable`. It builds against
the 1.5.378 packages and does not import the repository props.
- `docs/DeveloperGuide.md` and `.github/copilot-instructions.md` now
describe the central setting.

**Migration** (the 43 unset and 13 annotations-only projects, roughly
10k diagnostics)
- **Production code: annotated at the declarations.** Covers
`tools/Opc.Ua.SourceGeneration.Core`, `src/Opc.Ua.Lds.Server` and the
LDS sample.
- Members, parameters and returns are nullable where null actually
flows.
- Null-check helpers carry `[NotNullWhen]` (`IsNull`, `IsValidXmlName`,
`IsMethodTypeDesign`).
- `CoreUtils.Format`, `Utils.Format`, `ITemplateWriter.WriteLine` take
`params object?[]`. `Template.AddReplacement` takes `string?` (rendering
already skips null replacements).
- `IRegisteredServerStore.RegisterAsync` returns
`Task<RegistrationEntry?>`: it returns null when a registration is
removed.
- Dereferences backed by an invariant (for example, the validated model)
use `!`, so runtime behaviour is unchanged.
- **Behavioural changes in production code** (small, deliberate):
- `NodeSetToModelDesign` throws `InvalidOperationException` when the
file is not a NodeSet. Before, it threw a NullReferenceException on the
next line.
- `ModelDesignValidator` starts its lookup tables empty instead of null.
A redundant `if (m_nodes == null) return;` guard is gone.
- **Test code: minimal annotations.**
  - `?` on values that can be null, `!` where the test asserts non-null.
  - `= null!` for fixture members assigned in SetUp.
- `TryGetStructure<T>(out T? x)` where the inferred `T?` broke the
`IEncodeable` constraint.
- A few redundant `== null` checks on structs (`NodeId`, `ArrayOf<T>`)
folded into the adjacent `IsNull`/`Count` checks.

**Notes for reviewers**
- The diff is large (~1,500 files) but almost entirely mechanical: `?`,
`!` and directive removal. The non-annotation changes are listed above.
- Known follow-up, not done here:
`IModelDesign.Namespaces/TargetNamespace/NamespaceUris` stay
non-nullable even though the validator returns null before it loads a
dictionary. Making them nullable would ripple through every generator
call site.

## Related Issues

- Fixes #4439

## Checklist

- [ ] I have signed the
[CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf)
and read the
[CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md)
doc.
- [ ] I have added tests that prove my fix is effective or that my
feature works and increased code coverage.
- [x] I have added all necessary documentation.
- [x] I have verified that my changes do not introduce (new) build or
analyzer warnings.
- [ ] I ran **all** tests locally using the **UA.slnx** solution against
at least .net **framework** and .net **10**, and all passed.
- [ ] I fixed **all** failing and flaky tests in the CI pipelines and
**all** CodeQL warnings.
- [ ] I have addressed **all** PR feedback received.

The "no new warnings" box covers net10.0 and net48: full non-incremental
builds of `UA.slnx`, `tools/SourceGeneration.slnx`,
`tools/MigrationAnalyzer.slnx` and the Aot test projects finished with 0
warnings and 0 errors under the repository's warnings-as-errors
settings. net9.0 and net8.0 also had no nullable diagnostics, but they
were not rebuilt after the final fixes. Tests have not been run locally
yet.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
 2026-10-08 08:48:13 +02:00
romanett 2ed505547b Fix flaky CancelledRecoveryCanBeReplacedWithoutSuppressingLaterKeepAlive test (#4596) 
## Proposed changes


`ManagedSessionReconnectTests.CancelledRecoveryCanBeReplacedWithoutSuppressingLaterKeepAliveAsync(False)`
failed on #4594 (a workflow-YAML-only PR) in [run
36590762260](https://github.com/OPCFoundation/UA-.NETStandard/actions/runs/36590762260),
`test-windows-net10.0 (5/30)`:

```
Assert.That(harness.Logs.Records.Any(
    record => record.EventId.Name == "ManagedSessionKeepAliveFailureSuppressedWhile"), Is.False)
  Expected: False
  But was:  True
```

### Root cause (test race, not a product bug)

The suppressed record doesn't come from the failed keep-alive the test
checks. It comes from the recovery that this failure starts:

1. The scripted keep-alive read returns `BadNoCommunication`.
`ManagedSession.OnInnerKeepAlive` does not suppress it and calls
`TriggerReconnect`, as expected.
2. The outer recovery reactivates through the channel manager. When
`Session.ReconnectCoreAsync` finishes, `StartKeepAliveTimerAsync`
restarts the keep-alive timer with due time 0, which sends a read right
away.
3. The test reset `KeepAliveStatus` to Good only in the continuation
after `FailedKeepAlive`. That continuation runs asynchronously, so the
new read can still return Bad.
4. `ChannelEntry` goes `TransitionTo(Ready)` → `SignalReady()` →
`CompleteParticipantRecoveryAsync`, and only releases
`m_reconnectCoalescer` afterwards. A read released by `SignalReady` that
fails inside that window sees `ChannelRecoveryInProgress == true`.
Suppressing it there is intended: the manager is still restoring behind
its provisional Ready gate.

The assertion scanned every log record, so it also caught this correctly
suppressed failure from the follow-up recovery.

**Proof of causality:** adding `Task.Delay(100)` after `SignalReady()`
in `ChannelEntry`, plus `Task.Delay(50)` before the test reset
`KeepAliveStatus`, made both test cases fail every time with the CI
assertion. The log dump showed `KeepAliveReadFailed` →
`ManagedSessionKeepAliveFailureSuppressedWhile` right after the second
recovery's `Reactivating → Ready`. Both delays have been removed.

### Fix

- Check the suppression decision synchronously in a `KeepAlive` handler.
`ManagedSession` raises that event right after it decides whether to
suppress. The assertion now covers only the failure under test.
- Reset `KeepAliveStatus` to Good in that same handler, before the
triggered recovery can send another read.

## Types of changes

- [x] Bugfix (non-breaking change which fixes an issue)

## Checklist

- [x] I have read the
[CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md)
doc.
- [x] I have signed the
[CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf).
- [x] I ran tests locally with my changes, all passed.

Local verification (net10.0):
- With the product-side repro delay still in place, both cases pass with
the fix.
- `ManagedSessionReconnectTests` (17 tests): 30 sequential runs, all
green.
- `CancelledRecoveryCanBeReplacedWithoutSuppressingLaterKeepAlive*`: 64
runs across 8 parallel workers, all green.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
 2026-09-30 03:44:18 +02:00
Marc Schier c4e0721a29 Add a documentation learning path and fix issues found while verifying it (#4555) 
## Summary

### Documentation
- Stage the documentation index as a learning path: a first application,
independent client, server, and PubSub paths, then data and services,
companion models, production, advanced integration, and draft or
experimental capabilities.
- Add Getting started (repository samples or NuGet packages), OPC UA
concepts, Your first information model, and a production readiness
checklist.
- Add Native AOT application publishing, a first PubSub publisher and
subscriber exchange, and the UDP transport options.
- Add or expand contents navigation throughout `docs/`, and keep
contributor documentation separate.
- Correct outdated SDK examples and capability guidance, consolidate
repeated explanations into canonical guides, and repair local links and
heading structure.
- Rewrite What's New in 2.0 as a concise feature overview with short
linked details; keep migration instructions in the Migration Guide.
- Expand the feature catalogue with specification IDs, GDS, LDS, the OPC
10000-21 onboarding registrar, ISA-95, xRegistry, OpenUSD, and
sample-local models; distinguish draft status, explain crypto
pluggability, and bring performance guidance forward.
- Address review feedback on wording, active monitored items during
shadow reload, and the `Samples.md` filename and references.
- Regenerate the `opcua-v20-migration` plugin's bundled copy of the
migration guides so it matches the edited `docs/migrate/2.0.x` guides.
- Merge the latest `master`, preserving upstream resource-isolation
guidance within the task-based documentation index.
- Retain deferred Lingity findings where appropriate; this PR does not
claim that every document meets the Lingity score thresholds.

### Fixes found while verifying the guides
Each fix has regression tests that fail without it.
- **Client:** V2 subscriptions compared their lifetime with the
operation timeout instead of the session timeout, and logged a spurious
warning. `ISubscriptionEngineContext`, which is new in 2.0, gains a
`SessionTimeout` property; implementations must add it.
- **PubSub:** a subscriber now registers the `DataSetMetaData`
configured on its readers (OPC 10000-14 6.2.9.4), and the UADP decoder
decodes DataSetMessages that carry no ConfigurationVersion. Before, a
secured subscriber of the reference sample received no DataSets.
- **Source generator:** a ModelDesign method that declares its arguments
inline, without a method type, produced code that did not compile; it
now uses `MethodState`. The generated code of existing models is
unchanged.
- **MinimalClient sample:** waits for its first data-change notification
before it disconnects.

### Known issues, not addressed here
- UDP multicast: the publisher does not send on the interface that the
subscriber joins, and `MulticastLoopback` defaults to false. The PubSub
guide uses unicast for the exchange on one computer.
- The UDP metadata announcement is sent unsecured on secured writer
groups; a secured subscriber drops it with a warning.
- A server does not start in globalization-invariant mode, because
`ResourceManager` creates `CultureInfo("en-US")`. The Native AOT guide
cautions against the mode.

## Validation
- `git diff --check` and
`./.azurepipelines/validate-migration-plugin.ps1` (15 bundled migration
docs validated, plugin manifests synchronized).
- Local Markdown links, anchors, file-link casing, and contents lists
checked across all 87 documents under `docs/`. The index links every
top-level guide and every target of the previous index.
- Both Getting started routes, Your first information model, the PubSub
exchange, and the Native AOT publish of the Getting started client and
server were run as documented. The NuGet route and the tutorial use the
public `2.0.0-preview.5` packages.
- The changed libraries build for all target frameworks in Release
without warnings.
- Tests on .NET 10: Opc.Ua.SourceGeneration.Tests 199 passed;
Opc.Ua.SourceGeneration.Core.Tests 3,977 passed, 8 skipped;
Opc.Ua.Server.Tests `Fluent` category 621 passed; Opc.Ua.Client.Tests
subscription-engine tests 70 passed; Opc.Ua.PubSub.Tests 1,374 passed, 2
skipped; PubSub fuzz replay 145 passed. The new and affected tests also
pass on .NET Framework 4.8.
- Earlier merge validation: 174 resource-isolation tests passed on .NET
10.
- The full solution suite was not run locally.

Review and validation artifacts are maintained outside the repository.

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
 2026-09-28 19:19:39 +00:00
Marc Schier 46fa7d364a Harden recovery, concurrency, and resource lifetimes across the OPC UA stack (#4516) 
# Description

Bound managed-session recovery, preserve Publish workers and recovery
ownership, and address the client, server, transport, and certificate
audit findings and subsequent concurrency review. Also reduce the memory
retained by exhaustive session-test diagnostics.

## Recovery and Publish ownership

- Add `ManagedSessionOptions.ChannelReconnectTimeout` through direct
construction, the fluent builder, and DI. The automatic duration is
`max(3 * KeepAliveInterval, revised SessionTimeout, OperationTimeout)`,
sampled at cycle start. Positive overrides are supported;
`Timeout.InfiniteTimeSpan` explicitly opts out. Raw sessions retain
their unlimited default.
- Add `CreateReconnectBudget(TimeProvider)` as a required
`IReconnectParticipant` member, as agreed in review. Custom participants
implement it and may return null for no participant-specific bound.
- Enforce the earliest participant/caller deadline through transport
reconnect, activation/recreation, and subscription restoration. Late
callers can shorten an active cycle but cannot extend it.
- Cancel and unwind recovery, fence late responses, and release channel
recovery ownership before the existing `Faulted` event hands control to
the outer reconnect/failover policy. Preserve session recreate admission
through final Publish cleanup and keepalive suppression until shared
recovery is complete.
- Treat participant budget creation/evaluation failures, including
exceptions at the exact deadline, as terminal shared-recovery failures.
Notify participants before exposing `Faulted`, preserve the original
diagnostic, and never silently replace a finite budget with unlimited
recovery.
- Keep caller-supplied replacement channels, managed bindings, wrapper
event subscriptions, and recovery-owner identities consistent across
success, failure, and cancellation. Reject replacement during active
recovery and fence retired callbacks so they cannot clear newer
ownership or suppress later keepalive recovery.
- Retain eligible Publish workers through temporary
subscription-identifier clearing without retaining workers for
never-created or removed subscriptions. Preserve cancellation,
acknowledgement rollback, and drain-before-replacement ordering.
- Coordinate automatic subscription updates with session recovery:
cancel admitted updates, keep them paused across a failed-restoration
handoff, and retry only after explicit restoration completes. This
prevents competing CreateSubscription requests and updates against a
retiring session, while preserving manual recreation, transfer fallback,
and pending option changes.
- Exercise the real ManagedSession, V2 subscription engine, and channel
ready gate with scripted transports and an injected clock. Cover
transfer fallback, deadline handoff, late completions, and resumed
notification delivery.

## Audit and review corrections

- Correct certificate/trust-list transactions, change-pump lifetimes,
validation, peer revalidation, and reset/resource ownership.
- Harden node admission, attribute/range/dimension validation,
monitored-item sampling and lifecycle transitions, event fanout, and
server shutdown. Run lifecycle readiness and compensation outside shared
monitored-item admission, with stale-completion ownership checks.
- Close synchronous node-manager admission before teardown and retain
resources until admitted service, lifecycle, and asynchronous method
work returns. Dispose the monitored-item manager outside both locks,
reject late calls before they can reach it, and forward asynchronous
disposal through the adapter so the master awaits the same completion.
Preserve reentrant shutdown initiation, cancellation/failure unwinding,
and shared cleanup-failure reporting.
- Bound WSS pending-request retention using connection-local wire
handles while preserving caller-visible request handles and translating
Cancel targets. Safely ignore retired replies and drain admitted
send/close users before resource disposal.
- Clarify that the shared `ClientBase` caller-handle sequence is
distinct from the non-wrapping WSS wire allocator. Existing
exhaustion/reopen, late-reply, and Cancel-translation regressions verify
the connection-local contract; no allocator behavior was changed by this
clarification.
- Preserve virtual-node validation and resolver service errors instead
of converting them to cached misses. A mismatched NodeId reports
`BadNodeIdInvalid`; callback type mismatches and resolver service errors
keep their exact status. Per-item boundaries preserve unaffected reads,
writes, history operations, method calls, browse paths, and
monitored-item creation/restoration, including permission-validation
paths. Caller cancellation still propagates; stored-item validation
failures are logged without aborting later restores.
- Correct client file/role/state-machine/streaming APIs, node-cache
behavior, redundancy/failover ownership, and session
configuration/identity handling.
- Cancel each owned peer-discovery scope before disposing it, including
when the bounded wait wins a cancellation-delivery race. Keep late
results out of the endpoint cache and preserve single-flight lookup
ownership.
- Keep materialized alias Browse results synchronized with the live
store by default, preserving explicit startup-only opt-out. Apply
HistoryUpdate service results per value while retaining explicit
all-or-nothing provider APIs.
- Update the directly related feature documentation and add focused
positive, negative, concurrency, and ownership regressions.

## Latest review-feedback corrections

- Fence stale failed lease observations with a successful-confirmation
generation distinct from the revocation epoch. Snapshot it before each
store read, including failed-CAS revalidation, so an old failure cannot
revoke or reschedule a newer lease while fresh ownership loss still
revokes leadership. Preserve independent overlapping acquisitions,
expiry, disposal, and late-success protections.
- Isolate attributable WSS response-decoding errors to their request,
including retired handles. Keep admitted socket sends connection-owned
so caller timeout/cancellation cannot abort sibling requests; shutdown
still cancels and drains those sends.
- Restart keepalive and classic Publish replenishment after successful
session reuse. Retire V2 worker demand after intentional
deletion/disable without confusing that state with temporary recovery
resets.
- Browse virtual directories lazily across continuation pages without
the 1,024-entry snapshot cap. Preserve active-page cancellation, bounded
metadata retention, pushback, and exactly-once asynchronous cursor
cleanup.
- Apply certificate/CRL staging in last-operation order, preflight
required issuer storage before writes, select actual policy-compatible
RSA keys for None endpoints, frame certificate chains independently of
signature OIDs, and preserve admitted validation results during
rejected-writer shutdown.
- Preserve base asynchronous disposal when a legacy override omits its
base call; avoid double derived disposal in the adapter. Let genuinely
admitted nested work finish while rejecting stale captured contexts and
returning null from closed lookup APIs.
- Re-park recovered monitored items if AddNodes rolls back. Give event
compensation a fresh finite deadline, and preserve healthy
Browse/Translate siblings at every reference/path boundary while
retaining starting-node errors and caller cancellation.
- Give final reconnect notifications a live, bounded shutdown-linked
lifetime. Synchronize child state before parent lifecycle handlers
outside the parent transition lock.
- Document the accepted existing policies: direct reverse connections
are single-use without a ReverseConnectManager, and multi-node stream
capacity is shared rather than reserved per node.

The expanded readiness controls also exposed a real generation-handoff
race: an old source could finish draining after activation was deferred
but before readiness tasks were collected, causing the wanted
replacement to be omitted from the wait set. A deterministic test gates
that exact interval; the fix keeps the replacement in the readiness
wait. The original race passed 200 repetitions after the fix. A separate
control now awaits both producer readiness and iterator entry under the
same original deadline, rather than treating those independent signals
as equivalent. Temporary logging and repetition are removed.

## CI integration contracts

The History and Server CI failures exposed integration tests that still
asserted the behavior intentionally changed by the audit fixes. A
Windows WoT negative test raced its caller deadline against upstream
retries. A subsequent Ubuntu Client failure exposed a real
automatic-update versus explicit-restoration race:

- History insertion collisions now verify exact per-value statuses and
all persisted timestamps/values, including preservation of the existing
conflicting entry. Explicit atomic-provider rollback controls remain.
- Alias materialization tests wait for actual publication and verify the
added node plus forward/inverse `AliasFor` references, instead of racing
a startup-only snapshot expectation. Explicit refresh opt-out controls
remain.
- The unavailable-upstream WoT test uses an explicit finite retry policy
through existing DI configuration, preserving its original four-minute
guard and requiring a mapped-value service error before caller
cancellation. Production retry defaults remain unchanged.
- The Client fix retains the exact one-create and late-response
assertions. Three deterministic real-subscription/real-manager cases
cover paused creation, disposal while paused, and cancellation of an
admitted create followed by explicit restoration and a pending option
update. Both composed restoration-deadline cases also prove automatic
updates stay blocked at the outer handoff and resume after restoration.
- The merged-head net48 correction prevents lost peer-resolver
cancellation and distinguishes the harness's delayed initial `Connected`
event from actual outer recovery completion. A deterministic timer-order
test covers both cooperative and cancellation-ignoring resolvers,
cancellation before return, rejection of late cached results, and a
fresh subsequent lookup.
- The remaining CodeQL alert was a false content flow through
`ArrayOf.Count` into an integer-only trust-list summary. Projecting the
same count through `Span.Length` keeps cardinality distinct from
serialized contents without allocating, dropping diagnostics,
suppressing the rule, or dismissing the alert. Five log-contract cases
cover null/empty/nonempty results and mixed chain-capable/fallback
listeners, asserting the exact count, message, and structured
properties.
- Classic transfer acknowledges messages selected for asynchronous
Republish only after their responses arrive. The existing locked
replay-selection decision returns the immediate-acknowledgement subset
to Session, preserving the public transfer API, advertised availability,
and immediate acknowledgement of older gapped messages not selected for
replay. A callback changing `RepublishAfterTransfer` cannot change the
already-selected acknowledgement ownership.
- Transfer integration tests explicitly control acknowledged versus
retained origin messages. Exact assertions distinguish retained replay
from requested fresh initial values by sequence number, client handles,
values, status, and source timestamp. Atomic counters, observed
delivery/keepalive progress, and failure-safe session/file cleanup
replace timing assumptions and racy resets.
- Server lifecycle tests verify the documented independent five-second
compensation deadline rather than requiring a non-cancellable token.
They cancel the actual request, prove compensation and unrelated
monitored-item creation still proceed one tick before the deadline,
preserve the original failure and ownership assertions, and verify the
completed deadline is retired.
- Synchronous and asynchronous directory-browser tests require the exact
provider exception instead of an empty-directory success, then verify
the failed cursor is not enumerated again. These correct expectations
left behind by the intentional, documented provider-error propagation
change; production behavior is unchanged.

No tests were disabled, assertions loosened, coverage thresholds
lowered, or CI checks made optional. Existing platform exclusions and
Explicit debug variants remain.

## Related Issues

- Fixes #4514
- Fixes #4515
- Builds on merged #4503 and retains the #4508 publishing-drain
behavior.

## Validation

**Lease-election review correction
(`a13afa641182c70983e16bbcfebf507e666a45a5`):**

The correction is based on the existing upstream merge
`43079e2b851fe338a4082066f8d14abfdafb6e32`. Three deterministic
stale-verdict
cases fail on the original source and pass with separate confirmation
fencing.
Six new cases cover the initial foreign-owner verdict, failed-CAS
revalidation
in both invocation orders, and fresh ownership loss through both read
paths,
including revalidation begun after another successful confirmation.

| Target and configuration | Passed | Failed | Skipped |
| --- | ---: | ---: | ---: |
| net10.0 Debug | 120 | 0 | 0 |
| net48 Debug | 120 | 0 | 0 |

The selection is the full `Opc.Ua.Core.Tests.Redundancy` namespace,
including
all 38 `SharedStoreLeaseElectionTests` cases and the existing expiry,
clock
rollback, disposal, and overlapping-success regressions. New witnesses
are
`StaleForeignLeaseReadDoesNotRevokeNewerAcquisitionAsync`,
`StaleFailedRevalidationDoesNotRevokeNewerAcquisitionAsync`, and
`FreshForeignLeaseObservationRevokesLeadershipAsync`. They assert
resulting
leadership, exact transitions, independent progress, and the unchanged
expiry
boundary. Retaining the initial generation for the fallback read
independently
makes the fresh-loss-after-confirmation control fail; that mutation was
removed
before the final rebuilt runs.

The successful scoped command is
`dotnet test tests\Opc.Ua.Core.Tests\Opc.Ua.Core.Tests.csproj -c Debug
--no-restore --filter
"FullyQualifiedName~Opc.Ua.Core.Tests.Redundancy"`,
with `CustomTestTarget` and `-f` matching each row and that framework's
assets
restored. Both changed C# files pass formatting and analyzers through
informational severity on both frameworks. These are scoped results, not
a
full-solution test or measured coverage-delta claim. CI monitoring is
paused
for this feedback-only operation; no current-head CI success is claimed.

**Latest CI contract correction
(`1cd1f2047bb5b50c9bf300b1ff710b0b2e935a29`):**

All four failing Server shard 23 jobs in
[run
36120946740](https://github.com/OPCFoundation/UA-.NETStandard/actions/runs/36120946740)
reported the same six cases. Two local replays reproduced all six
failures.
The source changes and documentation from `cd43b831a` establish both
intentional
contracts: independent bounded event compensation and surfaced provider
errors.
Only the two test files are corrected; production code and CI policy are
unchanged.

| Target and configuration | Passed | Failed | Skipped |
| --- | ---: | ---: | ---: |
| net48 Debug | 113 | 0 | 0 |
| net10.0 Release, CI Coverlet settings | 113 | 0 | 0 |

The selection contains `MonitoredItemManagerLifecycleTests`,
`DirectoryObjectStateTests`, `DirectoryBrowserBoundedEnumerationTests`,
and

`EventCompensationReadinessHasFiniteDeadlineWithInfiniteSourceTimeoutAsync`.
Both changed files pass formatting and analyzers through informational
severity
on both frameworks. Independent mutations restoring a non-cancellable
compensation token and swallowing provider errors fail all four
lifecycle
cases and both browser cases, respectively. Both mutations were removed
before
the final passing runs and commit.

The scoped command is `dotnet test
tests\Opc.Ua.Server.Tests\Opc.Ua.Server.Tests.csproj`
with `CustomTestTarget` and `-f` set to the selected framework,
`--no-restore`, and
`--filter
"FullyQualifiedName~MonitoredItemManagerLifecycleTests|FullyQualifiedName~DirectoryObjectStateTests|FullyQualifiedName~DirectoryBrowserBoundedEnumerationTests|FullyQualifiedName~EventCompensationReadinessHasFiniteDeadlineWithInfiniteSourceTimeoutAsync"`.
The modern run additionally uses `-c Release /p:CollectCoverage=true
--collect "XPlat Code Coverage" --settings
tests\coverlet.runsettings.xml`.
These are scoped results, not a full-solution test or measured
coverage-delta claim.
Hosted checks for that commit were pending at publication. The fail-fast
CI
watch was stopped when the requested operation switched to review
feedback;
no current-head CI success is claimed.

**Earlier master merge (`8b15ea6ef9fc2214ac8ec886f373bb3846dd6d6f`, base
`b4136d8e9bab0df680f41d9ef985f519160534b1`):**

The `TcpListenerChannel` conflict preserves both lifetime
responsibilities:
disposal clears the retained peer-certificate chain and unregisters the
quota-scoped message-assembly scheduler. Both fields and all upstream
deadline
handling remain intact. The only additional manual source adjustment
adds the
parentheses required by the existing analyzer around the session-count
fallback.

Master advanced during validation. The initial transport merge
`80303c434ce813cf75aa7fe54a6a32fe2aeafc30` was retained, and the newer
base was
merged normally in `8b15ea6ef9fc2214ac8ec886f373bb3846dd6d6f`, without
rewriting
history. The additional recovery, fluent-pipeline, and test-framework
overlaps
were inspected; the branch's cancellation, validation-error, and
acknowledgement
ownership fixes remain alongside the upstream changes.

Full `UA.slnx` builds passed on net10.0 and net48 with **0 warnings and
0 errors**.
The final scoped tests rebuilt against the committed tree
`2074f8efcb61d8d9c6106e7d6f76ee941940e358`:

| Target | Passed | Failed | Skipped |
| --- | ---: | ---: | ---: |
| net10.0 | 2,653 | 0 | 0 |
| net48 | 2,529 | 0 | 7 |

The net48 skips are existing platform exclusions: six RSA_DH_AesGcm
nonce cases
and one ClientWebSocket certificate-validation case requiring .NET 7 or
later.
The selection covers transport cleanup, reassembly
budgets/deadlines/admission,
peer-chain ownership, server session binding, fluent lifecycles and
source
generation, reconnect and keepalive, transfer acknowledgements, Publish
queues,
and JSON encoding. Modern-.NET WSS transport and real-server integration
cases
also pass. The resolved file passed solution-relative formatting and
analyzer
verification through informational severity on both frameworks.

These are scoped test results, not unfiltered full-suite or
measured-coverage
claims. Hosted CI was not monitored or repaired as part of this
merge-only
operation.

Selected local results below are overlapping validation runs, not
distinct-test totals.

**Earlier review-feedback batch
(`cd43b831a8f11de3f7d65c3681712a61e3c669a5`):**
The final net10.0 scoped selection passed **471 tests, with no failures
or skips**.
The final net48 solution-root selection passed **439 tests, with no
failures and
one unchanged platform skip**
(`ValidateServerCertificateFallsBackToTlsPolicyErrors`,
which requires newer ClientWebSocket certificate-validation support).
Both full nonincremental solution builds completed with **0 warnings and
0 errors**,
and all **40 changed C# files** passed informational-level
formatting/analyzer
verification. Both frameworks validated the identical final tracked
diff.
The focused matrix covers every one of the 20 current review threads,
including
real mid-frame WSS cancellation, stale admission contexts, directory
continuation
ownership, multi-element Browse/Translate failures, and the readiness
drain race.
The net10.0 solution-root test command has a separate pre-existing
runner conflict:
three unrelated AOT projects reject the VSTest target under
Microsoft.Testing.Platform
2.4.0 and SDK 10. Per-project net10.0 results are recorded separately;
no runner
migration, test exclusion, assertion relaxation, or CI-policy change is
included.
These are bounded selections, not unfiltered full-suite or measured
coverage claims.
At publication of `cd43b831a`, hosted CI was not green: the
`OPCFoundation.UA-.NETStandard` check reported `ACTION_REQUIRED`,
AppVeyor reported
`FAILURE`, and the two signed-build jobs were in progress. No CI jobs
were rerun,
cancelled, approved, or repaired as part of that feedback-only
operation.

**Classic transfer correction
(`f425376fca045794122e19aaf9cafebc4a717d91`):**
The preceding head's
[macOS net10.0 shard
25/30](https://github.com/OPCFoundation/UA-.NETStandard/actions/runs/35980326299/job/107570512458)
failed
`TransferSubscriptionTest(DisconnectedRepublish).TransferSubscriptionAsync(True,False)`
at the static-node count assertion: expected 10, received 20. The trace
contains a successful,
explicitly requested republish of retained sequence 1 and a fresh
initial-value batch at sequence 3,
not duplicate dispatch of one response.

Controlled lost origin acknowledgements reproduced that incorrect test
expectation on both the
preceding PR head and exact master base
`a2539cb53dd1666cef856f33812d7a8830d68909`.
The maintainer explicitly authorized correcting this existing issue in
the PR. Retained replay
and requested initial values are independent, as specified by
[OPC UA Part 4,
TransferSubscriptions](https://reference.opcfoundation.org/specs/OPC-10000-4/5.14.7).
The reason that the original hosted run retained its particular origin
message remains unproved.

The expanded full-suite run then exposed a **real production race**, not
just a count assertion:
Session queued acknowledgements for all transferred available messages
before the asynchronous
Republish responses arrived. Publish could delete those messages first,
causing
`BadMessageNotAvailable`. A gated real-Session/classic-engine regression
failed both replay-enabled
cases on the original implementation while both no-replay controls
passed. The ownership fix
passes all 24 combinations of empty/consecutive/unsorted-gapped queues,
replay preference,
sequential delivery, and a reentrant preference change.

All 40 normal real-server transfer cases pass on both frameworks.
Independent mutations
suppressing actual replay and duplicating observed sequence records each
fail the intended
assertion; both mutations were removed. The full classic suite passes on
both frameworks.
Hosted CI was pending when that correction was published; the earlier
failed hosted job was not
rerun for a lucky pass.

**Local net48 caveat:** the full Client run timed out in the
pre-existing fake-clock test
`ReconnectAsyncSwapsFaultedLeaseEntryAsync`, which does not exercise
subscription transfer.
Its unchanged isolated replay passed, but that does not resolve the
full-suite timeout.
No unrelated code or assertions were changed, and a clean full Client
net48 run is not claimed.

| Scope | net10.0 | net48 |
| --- | --- | --- |
| Full Client.Tests after transfer acknowledgement correction | 2,549
passed | 2,552 passed, 1 timeout failure noted above, 1 existing
platform skip |
| Full classic-subscription project after transfer correction | 57
passed, including all 40 normal transfer cases | 57 passed, including
all 40 normal transfer cases |
| Gated transferred-message acknowledgement ownership | 24 passed,
included above | 24 passed, included above |
| Saved-session consumer of the shared subscription helper | 40 passed |
16 passed, 24 existing unsupported-security-policy skips |
| Audit regression matrix | 2,339 passed | 1,934 passed, 34 existing
skips |
| Full Client.Tests after review follow-ups | 2,520 passed | Focused
selections only |
| Full Client.Tests after CI recovery correction | 2,523 passed, CI
coverage collector | 2,527 passed, 1 existing platform skip |
| Latest merged-head recovery/redundancy selection | 139 passed | 139
passed |
| Full Client.Tests after merged-head net48 correction | Focused
selection above | 2,529 passed, 1 existing platform skip |
| Lifecycle and actual-dimension regressions | 146 passed | 146 passed |
| Focused recovery follow-ups | 7 passed | 7 passed |
| WSS-related fixtures | 93 passed | Not applicable |
| CI History integration corrections | 28 passed | 28 passed |
| CI alias, dispatcher, and explicit-atomic controls | 63 passed | 63
passed |
| Full WoT sample project with CI coverage collector | 29 passed | Not
applicable |
| Trust-list effects and count-only summary contract | 22 passed | 22
passed |
| Virtual-node feedback, node-manager, dispatcher, and lifecycle
selections | 661 passed, 63 existing not-executed cases; all 48
virtual-node cases passed | 661 passed, 63 existing not-executed cases;
all 48 virtual-node cases passed |
| WSS wire-handle exhaustion, late replies, and Cancel translation | 9
passed | Not applicable |
| Synchronous node-manager disposal and related service/lifecycle suites
| 1,037 passed, 64 not-executed cases from existing fixture assumptions;
all 93 sampling/disposal cases passed | 1,037 passed, 64 not-executed
cases from existing fixture assumptions; all 93 sampling/disposal cases
passed |

- Full solution builds passed on net10.0 and net48 during the audit
work. The subsequent full non-incremental net10.0 build also completed
with zero warnings/errors.
- Modified source/test formatting and analyzer checks passed through
informational severity.
- Original composed recovery regressions passed repeated runs on both
frameworks. Independent mutations removing Publish cancellation,
bypassing the recreate drain call, or ignoring ready-gate cancellation
each failed the intended assertion; all mutations were restored before
passing runs.
- The latest recovery correction passed 100 executions of each original
restoration-deadline scenario with unchanged assertions. Restoring
premature update admission independently made both strengthened handoff
cases fail; the correct retention behavior was restored afterward.
- Startup-event and cancellation-delivery timing probes reproduced the
subsequent net48 failures, then passed 100 executions of each of the
three scenarios with the fixes. All timing probes and repetition were
removed. The permanent timeout-order regression also went RED on the
original cancellation cleanup without sleeps or scheduling
amplification.
- Final post-format recovery/subscription/engine checks passed all 151
selected cases on net10.0. The first local full net48 attempt aborted in
`clr.dll` with native access violation `0xc0000005`; an
unchanged-assembly full diagnostic rerun passed as recorded above. That
native host crash has not been attributed or claimed fixed.
- A minimal replay with the same CodeQL CLI and query-pack versions
reproduced the custom-count alert and confirmed that the standard
span-length projection is clean while direct sensitive-value and
raw-identifier logging controls remain flagged. The source was already
logging only a count; no identifier or certificate disclosure was
established at the reported location.
- The virtual-node feedback regressions first reproduced five
error-preservation/cache failures; expanded service-dispatch cases
exposed two additional whole-batch failures before the
operation-boundary fixes. The final selections pass on both frameworks.
Genuine misses remain cached, successful results remain reusable, failed
validation allows same-cache retry, and request cancellation still stops
the batch.
- Three deterministic synchronous-manager disposal regressions
reproduced early resource release and lifecycle calls reaching a manager
during disposal. The fix passes direct and adapted calls, a
sampling-worker join after the last operation exits, reentrant shutdown,
cancellation and failure paths, repeated disposal, and exact rejection
across 39 service/lifecycle entry points both during drain and after
cleanup. The original outside-node-lock disposal regression remains and
now requires the worker to observe closed admission.
- Prior-head CI on `c985f8bded77a60449d9cd381c8a004d8590ccf6` completed
with **168 passing checks, one expected release-package-publication
skip, and no failures or pending checks**. All test shards, signed
builds, container builds, the coverage check, and the CodeQL PR check
passed. Hosted CodeQL analysis `1829262912` completed successfully and
marks count-flow alert 385 fixed. This is evidence for that head, not a
claim that subsequent review follow-ups have completed hosted CI.

### Session-test diagnostic memory

Complete browse listings are streamed to unique NUnit attachments rather
than retained in inline NUnit/VSTest output. Detached regression-test
results are cleaned up without deleting runner-collected attachments.

- Two covered session-suite runs each completed with 782 passed and 379
existing skips. Peak working set fell from 4.86 GiB to 2.44-2.78 GiB.
- TRX output shrank from 303.1 MiB to approximately 40.7 MiB, retaining
all 46 complete browse attachments.
- Seven output regressions cover exact content, empty/single/large
listings, disabled/enabled output, bounded inline summaries, and unique
filenames.

The separate Azure build-18452 VM deprovisioning cause remains unproved
without provider logs. The memory measurements do not establish its
cause.

Unfiltered local `UA.slnx` tests, including the long stress/soak suites,
have not been run on both frameworks. A local coverage delta was not
collected; builds and focused tests are not substitutes for those
claims.

## Checklist

- [ ] I have signed the
[CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf)
and read the
[CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md)
doc.
- [ ] I have added tests that prove my fix is effective or that my
feature works and increased code coverage.
- [x] I have added all necessary documentation.
- [x] I have verified that my changes do not introduce (new) build or
analyzer warnings.
- [ ] I ran **all** tests locally using the **UA.slnx** solution against
at least .net **framework** and .net **10**, and all passed.
- [ ] All checks on the latest review-follow-up head have completed
successfully; prior-head CI passed as recorded above.
- [x] I have addressed **all** PR feedback received.

Tests have been added and pass as recorded above; the hosted coverage
check passed on the prior head identified above. The combined
tests/coverage-increase checkbox remains open because no local coverage
delta was measured.

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 68f26810-22b8-476f-abcd-0a8c82d6e208
 2026-09-25 14:59:34 +00:00
romanett a451053758 Fix the recurring and permanent CI failures on master (#4446) 
# Description

Almost every `master` build was reporting yellow or red, which meant the
pipeline signal had stopped being useful: a genuinely broken build
looked the same as a healthy one. I triaged every `master` build from
the last week across Azure DevOps (definition 14, 38 builds) and GitHub
Actions, clustered the errors by root cause, and fixed the nine that
were still live.

The headline finding is that the two loudest problems were not test bugs
at all. Codecov rejects the upload on **28 out of 28 builds**, and that
single warning is what turned every master build `partiallySucceeded`.
Separately, the weekly HA long-haul soak was mathematically incapable of
passing: the configured duration is per *scenario*, and the two
`[NonParallelizable]` soaks run back to back, so the floor is 2 x 60
minutes plus teardown against a 110-minute step budget.

A tenth cluster (the `Opc.Ua.Sessions.Tests` reverse-connect flake, 6
hits around Aug 31) was already fixed by #4366 and has not recurred, so
it needed no work.

## Clusters and root causes

| Cluster | Frequency | Root cause |
|---|---|---|
| codecov upload exits 1 | **28/28 builds** | Codecov returns
`Repository not found`; the resulting warning made every master build
yellow |
| `No space left on device` in coverage merge | nightly | Hosted Ubuntu
agent disk exhausted by the per-matrix coverage artifacts |
| Sample HA Long-Haul timeout | weekly | 2 soaks x 60 min against a 110
min budget, so it could never pass |
| `Artifact coverage-report already exists` | on stage re-run | Pipeline
artifact names must be unique per build |
| `NodeCache*` -> `address already in use` | macOS | `ServerFixture`
retried only `BadNoCommunication`; Kestrel https throws
`IOException`/`AddressInUseException` and escaped the retry, failing the
whole `OneTimeSetUp` |
| `MetricsAreEmittedForChannelLifetimeAsync` | macOS | Entry teardown
emits `close` *before* the `active(-1)` up/down counter, but the test
only waited for `close` |
| `OnPublishReceivedKeepAliveShouldDispatchKeepAliveAsync` | net472 |
The processor queues the ack *after* dispatching the notification the
test waits on |
| `ACarriedPartIsAuthoredAtTheGripperItRidesInAsync` | macOS | A single
starved sample (0.2125 m) tripped a 0.20 m absolute worst-case bound |
| `RobotIntentLiveChannelTests` fixture timeout | net472 | 30 s startup
budget too tight, and the failure text interpolated `lastException`
*before* the polling that assigns it, so it always read `Last error: .`
|

## Approach

**Pipeline.** Coverage reporting is best-effort by design, so the
codecov step now swallows the uploader's exit code and reports the
failure as plain log output rather than a warning that colours the run.
The coverage merge job reclaims the unused Android/GHC/Swift/CodeQL
toolchains before anything writes to the agent, and qualifies its
artifact name on a retry only, so the normal first attempt still
publishes `coverage-report`. The long-haul budgets were raised above the
real floor in both the workflow and the ADO pipeline, and the display
names now say `min/scenario` so the next reader does not repeat the same
miscalculation.

**Tests.** Each test fix targets the specific ordering or bound that was
wrong, rather than adding sleeps. The two metric/ack races now wait on
the measurement the assertions actually depend on. The OpenUSD test
gates the 95th percentile instead of the absolute worst case: the defect
it guards is systematic and moves *every* sample 0.25-0.49 m while the
robot drives, so it still fails that gate loudly, while one straddled
sample no longer can.

## Worth a careful look

- **The port-collision fix is the one real behavioural change.**
`ServerFixtureUtils.IsPortUnavailable` now classifies every transport's
form of the collision, including matching Kestrel's
`AddressInUseException` by type name so the test framework needs no
reference to the ASP.NET Core connection abstractions. It deliberately
does *not* match unrelated failures, which is covered by a negative
test, so a certificate error still surfaces immediately instead of being
retried 25 times on different ports.
- **I dropped an integration test I had written for that fix.** While
validating it I found the UA-TCP listener tolerates a partial bind on
Windows, so `StartAsync` succeeds on the same port and the test asserted
something untrue. I replaced it with deterministic unit tests of the
classifier instead of shipping a test that would itself become flaky.
- **The codecov fix is a mitigation, not a cure.** The `Repository not
found` response is account-side: the `CODECOV_TOKEN` is stale or the
repository is not activated. This change stops it from colouring the
build and logs the failure plainly, but uploads will not actually land
until someone refreshes the token. Flagging that explicitly so it is not
mistaken for a full fix.

## Related Issues

N/A - this addresses CI failures observed directly on `master` builds
rather than a filed issue.

## Checklist

_Put an `x` in the boxes that apply. You can complete these step by step
after opening the PR._

- [x] I have signed the
[CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf)
and read the
[CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md)
doc.
- [x] I have added tests that prove my fix is effective or that my
feature works and increased code coverage.
- [x] I have added all necessary documentation.
- [x] I have verified that my changes do not introduce (new) build or
analyzer warnings.
- [x] I ran **all** tests locally using the **UA.slnx** solution against
at least .net **framework** and .net **10**, and all passed.
- [x] I fixed **all** failing and flaky tests in the CI pipelines and
**all** CodeQL warnings.
- [ ] I have addressed **all** PR feedback received.

### Notes on the checklist

- Documentation: no doc updates were needed. The changes are CI
configuration and test-harness fixes with no public API surface, and the
rationale is captured in comments at each changed site.
- Local verification: all touched projects build clean on **net10.0**
and **net48**, and the targeted tests pass (Server 5/5, Client 46/46,
Features 31/31, Robotics.Intent 20/20, plus the Di and Core.Tests
cases). One pre-existing local `net48` PKI-store failure in
`Opc.Ua.Robotics.Intent.Tests` reproduces on unmodified `master` and is
unrelated to this change; CI's `net472` leg runs that project fine.

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Marc Schier <marcschier@users.noreply.github.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: marcschier <11168470+marcschier@users.noreply.github.com>
 2026-09-09 09:16:35 +00:00
Marc Schier 416d619c41 Split the CI matrix across both systems, split the required test gate from the advisory coverage check, and give background work an owner (#4158) 
# Description

Two changes to how CI is routed and how it reports.

## 1. Split the CI matrix back across both systems

Flips both halves of the CI backend switch from `ado` to `actions`, so
the workload is shared instead of concentrated on the managed pool. The
switch is checked into source in **two places that must always be
flipped together**:

| File | Setting | Before | After |
| --- | --- | --- | --- |
| `azure-pipelines.yml` | `parameters.ciBuildBackend` | `ado` |
`actions` |
| `.github/workflows/buildandtest.yml` | `env.CI_BUILD_BACKEND` | `ado`
| `actions` |

| Work | GitHub Actions | Azure Pipelines (managed pool) |
| --- | --- | --- |
| All-TFM solution builds | `build-all-tfm-windows` + `-linux` | two-TFM
PR gate (`net10.0`, `net48`) |
| Test matrix | ubuntu, every test project | Windows `net48` + `net10.0`
fast PR legs |
| Native AoT | ubuntu | Schedule/Manual only |
| macOS | always (no managed-pool macOS image) | — |

## 2. Split the required gate from the advisory coverage check

The coverage gate job was appended to *every* test stage, so `Fast PR
test` emitted three sibling checks whose display names collided —
`Coverage Release (net10.0)` appeared twice, once for the Windows leg
and once for macOS, with nothing to tell them apart:

```text
✓ Coverage Release (net48)      55s
● Coverage Release (net10.0)
○ Coverage Release (net10.0)
```

That job also fused two unrelated concerns: whether the tests passed
(which must block a merge) and whether coverage met its thresholds
(which should not — the `net10.0` leg had `enforceCoverage: true`, so a
coverage dip blocked merges).

Both concerns now get a stage of their own, mirrored in both CI systems:

| Concern | Azure Pipelines | GitHub Actions | In the branch ruleset? |
| --- | --- | --- | --- |
| Every test passed | **`Tests passed`** stage | **`build-and-test
summary`** job | **Yes — required** |
| Coverage meets thresholds | **`Code coverage`** stage | **`code
coverage`** job | **No — advisory** |

The coverage check reports a clean failure when the thresholds are
missed, so the miss is visible, but it never blocks the merge.

Coverage is now merged **once** across every leg of the run instead of
once per leg, which also removes the `enforceCoverage` /
`publishCoverage` "exactly one invocation may set this" parameters.
Neither check re-runs any tests — the previous design re-ran the whole
suite in one job with the collector attached, which serialised a suite
that is deliberately fanned out and always exceeded the stage timeout.

### Coverage checks now exist in GitHub Actions

`build-and-test` already ran with `--collect:"XPlat Code Coverage"` and
uploaded the fragments, but nothing ever merged or evaluated them. It
now runs the same `check-coverage.ps1` against the same
`coverage-thresholds.json` as Azure. Since `CI_BUILD_BACKEND` defaults
to `actions`, that ubuntu matrix — which runs *every* test project — is
the fullest coverage signal in the repository.

### Reporting the numbers

`check-coverage.ps1` gained `-SummaryPath`, rendering a markdown report
that both systems surface:

- **GitHub Actions** — appended to the job summary and upserted as a
single **sticky pull-request comment**, updated in place on each run.
Threshold misses also appear as `::error::` annotations.
- **Azure Pipelines** — attached to the build summary via
`##vso[task.uploadsummary]`.

The report is a pass/fail table of the project floor, patch coverage and
baseline delta, with the uncovered changed lines listed by file in a
collapsible block. Rendered example:

> ## Code coverage
>
> ❌ **Coverage gate failed.** This check is advisory and does not
block the merge.
>
> | Check | Result | Threshold |
> | --- | --- | --- |
> | ✅ Project line rate | **72.10%** (40 731/56 496
lines) | >= 70.00% |
> | ❌ Patch coverage | **0.00%** (0/1 changed lines) | >= 75.00% |

The comment is fork-safe: a `pull_request` from a fork gets a read-only
token regardless of the `permissions:` block, so it degrades to the job
summary alone. The sticky comment uses `actions/github-script` rather
than a community action, keeping this workflow on first-party actions
only.

## Notable correctness details

These were found and fixed during review, and are the parts most worth a
second pair of eyes:

- **`dependencies.<stage>.result` resolves at *stage* scope only.** At
job scope `dependencies` refers to other jobs of the same stage, so
reading stage results from a jobs template would have silently returned
empty and the gate would have passed everything. `gate.yml` and
`coverage.yml` are therefore **stage** templates that derive their own
`dependsOn` from the same stage list they evaluate, so the two can never
drift apart. An unresolved result is a hard error.
- **Both rollups run on `not(canceled())` / `always()`**, not on
success. A check that reports **skipped** is treated by GitHub as
**satisfied**, so a required check that skips when its dependency fails
would wave a red build straight through.
- **`get-matrix.ps1` used to emit an empty matrix on a discovery miss.**
The downstream job is then skipped, a skipped job rolls up as a
*succeeded* stage, and the gate would have approved a run that executed
no tests. It now fails, with an `-AllowEmpty` opt-out.
- **Zero coverage reports used to be a green advisory check**, which
would hide a broken collector indefinitely. Both systems now distinguish
"the tests never ran" (warning) from "the tests ran and published
nothing" (error).
- The gate reads stage-level results rather than enumerating
matrix-generated job names, which change whenever a test project or
agent is added.

## Test-host hang and the reconnect defect behind it — both fixed

Flipping `CI_BUILD_BACKEND` to `actions` started exercising the ubuntu
matrix, which
`master` never runs, and it immediately surfaced two real defects.

**1. The test host hung for ten minutes.** `Assert.ThrowsAsync` blocks
the calling thread
with no timeout, and these tests drive a *fake clock* from that same
thread — so once the
reconnect parked on a fake-clock timer, nothing could ever advance it.
NUnit sat in
`WaitForCompletion` until `--blame-hang-timeout` killed the host,
aborting the run at 3902
of 4238 tests and taking every other test in the assembly down with it.
A hang dump named
the culprit exactly. Replaced with an awaited, bounded
`AssertThrowsAsync` helper (the
repo's own rules forbid sync over async). The run now completes in
**~1m29s** with all
tests executed.

**2. A budget-aware reconnect never returned.** With the hang gone, two
tests failed with
`Expected ServiceResultException but got TimeoutException`. `dumpasync`
on a hang dump gave
the whole chain:

```
ReconnectAsyncWithBudgetShrinksDelayToFitRemainingAsync
  → AssertThrowsAsync
  → ClientChannelManager.ReconnectAsync
  → ClientChannelManager.ReconnectLeaseAsync
  → ClientChannelManager.SwapFaultedEntryAsync   ← parked here
```

`master` (via #4128) added `IsTerminalReconnectRace` so a reconnect that
loses a race
against a concurrent close recovers on a freshly swapped entry. The
predicate only asks
whether the entry ended up `Closed`/`Faulted` with
`BadSecureChannelClosed` — which is
*also* exactly how a reconnect ends when the caller's retry budget or
the reconnect policy
is deliberately exhausted. So a budget-exhausted reconnect was misread
as a race: the
manager swapped the entry and started a **second, unbudgeted** cycle,
waiting out the swap
back-off first. `ReconnectAsync(channel, budget)` therefore could not
return within its
budget, and under a fake clock it never returned at all.

The fix requires the budget to have room left before treating a terminal
failure as a race:

```csharp
return !ct.IsCancellationRequested &&
    !budget.IsExhausted &&
    sre.StatusCode == StatusCodes.BadSecureChannelClosed &&
    entry.State is ChannelState.Closed or ChannelState.Faulted;
```

The parameterless `ReconnectAsync(channel, ct)` overload passes an
unlimited budget, so
genuine race recovery is unchanged. Verified locally: Core 4152, Client
2122 and Sessions
776 tests all pass, and the full GitHub Actions PR run is green across
56 jobs.

Two smaller fixes came along the way, neither weakening an assertion:

- `ObservableFakeTimeProvider` waiters made **relative** ("N more timers
from now") rather
than absolute ("the Nth timer of the run"). Absolute numbering let an
earlier reconnect
consume the slots, so a waiter completed before the awaited timer
existed and `Advance()`
  fired nothing.
- The raw worker thread in `NodeStateHandlerConcurrencyTests` was a
**foreground** thread
whose stop flag was read without a barrier and set only on the success
path — a failed
assertion leaked a spinning thread that pins the process at exit. Now
background,
  `Volatile`, stopped in a `finally`, joined with a bound.
## Background work now has an owner

The reconnect hang above was one symptom of a pattern the codebase had
**26 times** across seven assemblies: a bare `_ = Task.Run(...)`. Each
one handed work to the thread pool and forgot it — nothing observed the
exception if it threw, nothing bounded how many ran at once, and
disposal raced them, so a component could finish tearing itself down
while work it started was still touching its fields. Two of those sites
already carried a standing `// TODO: Await the task completion in
shutdown`.

`BackgroundTaskScope` (in `Opc.Ua.Types`) gives them an owner: schedule
with `Run`, drain with `DisposeAsync`, or signal-only with `Dispose`
where the owner's teardown is synchronous and awaiting would be sync
over async. `Run` never blocks and never throws — which matters, because
most of these call sites went to the thread pool precisely to escape a
lock they were holding. 11 unit tests cover drain, cancellation,
exception observation, the concurrency cap, post-shutdown rejection and
idempotent disposal.

Two of the fixes were more than re-wiring:

- **`SessionManager` / `SubscriptionManager` loops were structurally
unawaitable.** Both return `ValueTask`, so `Task.Factory.StartNew(() =>
...)` handed back a `Task<ValueTask>` that completes at the first yield.
Storing it would not have helped — awaiting it would only have awaited
the *scheduling*. Both now go through `AsTask().Unwrap()`, matching the
neighbouring `StartConditionRefreshWorker`.
- **`ChannelAsyncOperation` was swallowing subscriber exceptions.** Its
`doNotBlock` branch had no try/catch while the inline branch beside it
did. It is per-operation with no owner to hang a drain on, so it gained
the missing handler rather than a scope.

The Kubernetes readiness endpoint keeps a purpose-built semaphore
instead: an unauthenticated endpoint needs a hard cap on concurrent
handlers, not just an owner.

### ⚠️ Breaking: `ISessionManager.Shutdown()` is removed

Replaced by `ShutdownAsync(CancellationToken)`. There is no correct
synchronous way to wait for the session monitor loop, so leaving the
overload would only have preserved the race. This mirrors
`ISubscriptionManager`, which already had `ShutdownAsync`. Migration
notes are in `docs/MigrationGuide.md`; only one in-repo implementer
existed.

Also converted the injected `object partitionLock` shared by
`LogicalSubscription` and `CompositeMonitoredItemCollection` to
`System.Threading.Lock`, per the repo rule.

## Patch coverage now scales with the size of the patch

A flat floor over changed lines punishes small changes for arithmetic:
two uncovered lines in a four-line fix reads as 50 % and failed an 80/5
floor — which is exactly how this branch earned a red coverage check for
a four-line predicate change. A check that fires on changes nobody
considers undertested is one people learn to ignore.

| Coverable changed lines | Floor | Below it |
| --- | --- | --- |
| 1 – 10 | 50 % | warning, still passes |
| 11 – 100 | 60 % | warning, still passes |
| more than 100 | 75 % | **failure** |

Only changes past the last band can fail. Below it the author still gets
a warning naming the uncovered lines. Bands live in `patch.bands` in
`coverage-thresholds.json` and each carries its own `enforced` flag.
Verified end-to-end against synthetic repositories at every boundary.

## Codecov is back, as reporting

Dropped in #4122 when the in-pipeline gate replaced it. Keeping the gate
was right, but dropping Codecov also lost the PR comment, the
file-by-file diff view and the trend. It returns with both statuses
`informational: true` — two gates with two sets of thresholds would
eventually disagree, and `coverage-thresholds.json` stays the single
source of truth.

The upload is switchable and can never fail a build: `enableCodecov`
(Azure) and `ENABLE_CODECOV` (Actions), both defaulting on, both skipped
when `CODECOV_TOKEN` is absent as on fork PRs. Each system uploads the
single merged report it already builds, under its own flag. The old
`codecov.io/bash` uploader was sunset, so Azure uses the current CLI.

## Follow-up actions for maintainers

1. **Update the branch ruleset.** The per-leg `Coverage Release (…)`
checks are gone. Azure reports checks as `<pipeline> (<stage> <job>)`,
so the exact names are:

   | Check | Require? |
   | --- | --- |
| `OPCFoundation.UA-.NETStandard (Tests passed Verify stage results)` |
**yes** |
   | `build-and-test summary` | **yes** (already required) |
| `OPCFoundation.UA-.NETStandard (Code coverage Merge and evaluate)` |
no |
   | `code coverage` | no |
2. **Re-calibrate the thresholds.** `coverage-thresholds.json` is still
seeded from the last Codecov reading (73.61 %). The two systems now
report *different* numbers by design — Actions merges every project on
ubuntu, Azure only the Windows fast-PR legs, and scheduled runs read
higher still because the Debug / .NET 8 / .NET 9 / netstandard stages
also contribute. Re-seed from the GitHub Actions figure.
3. **Note:** a failed advisory coverage stage still turns the whole
Azure run red, which the README build badge reflects on `master`. That
follows from wanting a visible red X on a threshold miss; say the word
if you would rather it were a warning.
4. **The Azure agent pool is the slowest thing here.** Several runs on
this PR sat queued for hours, and one build failed with `Insufficient
system resources` while writing an XML doc file — an agent exhaustion
error, not a code error. It passed on re-run. Worth raising against the
`netstandard` Managed DevOps Pool capacity separately.
## Related Issues

_No tracking issue; this is CI infrastructure work requested directly.
It builds on the switch introduced in #4122._

## Checklist

- [x] I have signed the
[CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf)
and read the
[CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md)
doc.
- [x] I have added tests that prove my fix is effective or that my
feature works and increased code coverage.
- No product code changed, so there are no unit tests to add. The new
logic was verified with purpose-built harnesses instead:
- `check-coverage.ps1` run for real against a merged Cobertura report on
all three paths — pass, threshold failure, and patch coverage with
uncovered changed lines (a scratch commit adding an uncovered method was
correctly resolved to `DataTypeException.cs:193`, reported 0 % patch
coverage and exited 1).
- The gate's accept/reject logic dry-run over `Succeeded` /
`SucceededWithIssues` / `Skipped` / empty / `Failed` / `Canceled` — 6/6
correct.
- `get-matrix.ps1` exit codes verified in clean processes: empty → 1,
normal → 0, `-AllowEmpty` → 0.
- All five pipeline/workflow files parse; both PowerShell scripts parse.
- Action versions checked against the GitHub API and pinned to current
majors.
- [x] I have added all necessary documentation.
- Rewrote the coverage section of `docs/DeveloperGuide.md` as *Required
checks and coverage*, covering which checks belong in the ruleset and
which must not, how coverage is measured, where the numbers appear, and
why the two systems disagree. Corrected the stale "BLOCKS" wording in
`coverage-thresholds.json`.
- [x] I have verified that my changes do not introduce (new) build or
analyzer warnings.
- CI YAML, two PowerShell scripts and documentation only; no compilation
involved.
- [x] I ran **all** tests locally using the **UA.slnx** solution against
at least .net **framework** and .net **10**, and all passed.
- Not applicable — no product code changed. `Opc.Ua.Core.Schema.Tests`
was run repeatedly on net10.0 (119 passed) to generate the real
Cobertura fragments used to validate the gate.
- [ ] I fixed **all** failing and flaky tests in the CI pipelines and
**all** CodeQL warnings.
- [x] I have addressed **all** PR feedback received.
- Feedback on the path-relevance diff (now a merge-base diff) and the
target-framework numbering in `docs/DeveloperGuide.md` is fixed in
`15b8e38df`.
- A review pass over the gate/coverage split produced the four
correctness fixes listed above, in `863eefc4d`.

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 26aa55a2-5731-4a4c-b61c-df2f7a785eca
 2026-08-07 06:56:05 +02:00
Marc Schier 5efba99dbd Replace legacy EventSource providers with source-generated logging (#4058) 
# Description

Replace the four stack-owned `System.Diagnostics.Tracing.EventSource`
providers with source-generated `ILogger` logging.

- Preserve the legacy provider category names, numeric event IDs, event
names, levels, message structure, and semantic structured fields.
- Remove `ClientTraceFlags.EventLog` and route `ClientTraceFlags.Log`
through the canonical `OPC-UA-Core` compatibility events.
- Keep ChannelManager activities and metrics while moving its diagnostic
events to the `Opc.Ua.ChannelManager` logger category.
- Replace EventListener-based unit and stress-test collection with
structured logger providers.
- Document the ETW removal, retained compatibility identities, and
migration path for existing consumers.

## Validation

- `dotnet build UA.slnx -c Release -p:CustomTestTarget=net10.0
-p:NuGetAudit=false`
- Focused Core, Client, and Server logging tests on `net10.0` and
`net48`
- Stress harness Release build and
`SessionRecoversFromAcceptButStallAsync`
- Direct AOT test application: 115 tests passed
- `dotnet test UA.slnx -c Release -p:CustomTestTarget=net48
-p:NuGetAudit=false`

The full `net10.0` `dotnet test UA.slnx` invocation is currently blocked
by the repository's existing mixed VSTest/Microsoft.Testing.Platform
configuration in `Opc.Ua.Aot.Tests`; the AOT project builds and its test
executable passes directly.

## Related Issues

- Related to #1262

## Checklist

- [ ] I have signed the
[CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf)
and read the
[CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md)
doc.
- [x] I have added tests that prove my fix is effective or that my
feature works and increased code coverage.
- [x] I have added all necessary documentation.
- [x] I have verified that my changes do not introduce (new) build or
analyzer warnings.
- [ ] I ran **all** tests locally using the **UA.slnx** solution against
at least .net **framework** and .net **10**, and all passed.
- [ ] I fixed **all** failing and flaky tests in the CI pipelines and
**all** CodeQL warnings.
- [x] I have addressed **all** PR feedback received.

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: b0d90b8a-cde9-4fca-9a75-84f4cecb227d
 2026-07-23 21:53:31 +02:00
Marc Schier 7cec2d614d Move repository files to the consolidated layout 2026-07-16 06:25:54 +02:00