Skip to content

Fix build failure: update SubscriptionManager.CompleteAsync callers to new (IMessageProcessor, uint, CancellationToken) signature - #4118

Closed
marcschier with Copilot wants to merge 2 commits into
masterfrom
copilot/fix-build-windows-all-tfm
Closed

marcschier with Copilot wants to merge 2 commits into
masterfrom
copilot/fix-build-windows-all-tfm

Conversation

Copilot AI commented Jul 30, 2026 •

Copy link
Copy Markdown
Contributor

PR #4114 changed IMessageAckQueue.CompleteAsync from (uint, CancellationToken) to (IMessageProcessor, uint, CancellationToken) to fix incorrect eviction of zero-id subscriptions, but did not update SubscriptionManagerCoverageTests.cs, causing CS7036 build failures in the build-windows-all-tfm CI job.

Changes

  • IMessageAckQueue.cs — updated CompleteAsync signature to (IMessageProcessor subscription, uint subscriptionId, CancellationToken ct)
  • MessageProcessor.cs — tracks last non-zero server-assigned id (m_lastServerId); passes this and m_lastServerId to CompleteAsync instead of the current (potentially zero) Id
  • SubscriptionManager.cs — implements new signature: identity-based O(1) removal via completing as IManagedSubscription; skips zero-id entries in subscription history
  • FakeMessageAckQueue.cs — updated fake to satisfy new interface
  • SubscriptionManagerTests.cs — updated existing call sites; added CompleteForUncreatedIdDoesNotEvictPendingSubscriptionAsync covering the zero-id eviction scenario
  • SubscriptionManagerCoverageTests.cs — fixed the two call sites that caused the CI failure

Checklist

  • I have signed the CLA and read the CONTRIBUTING doc.
  • I have added tests that prove my fix is effective or that my feature works and increased code coverage.
  • I have added all necessary documentation.
  • 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.

@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

…d failure

Update IMessageAckQueue.CompleteAsync signature to include IMessageProcessor
as first parameter, matching the PR #4114 intent. Update all callers:
- MessageProcessor.cs: store last server id, pass self to CompleteAsync
- SubscriptionManager.cs: use completing IMessageProcessor for O(1) lookup,
  skip zero-id history entries
- FakeMessageAckQueue.cs: update fake impl to match new interface
- SubscriptionManagerTests.cs: update existing tests + add new test for zero-id
- SubscriptionManagerCoverageTests.cs: fix the two calls causing CI failure
Copilot AI changed the title [WIP] Fix failing GitHub Actions job build-windows-all-tfm Fix build failure: update SubscriptionManager.CompleteAsync callers to new (IMessageProcessor, uint, CancellationToken) signature Jul 30, 2026
Copilot AI requested a review from marcschier July 30, 2026 04:25
@marcschier marcschier closed this Jul 30, 2026
@marcschier
marcschier deleted the copilot/fix-build-windows-all-tfm branch September 11, 2026 12:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants