Repository navigation
Conversation
This was referenced Oct 13, 2026
niemyjski
marked this pull request as ready for review
October 13, 2026 04:38
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #580. Cancel protected work immediately when work-item renewal reports lost ownership, reject leases that expire during slow collection acquisition, and await renewal shutdown before releasing the lease in the documentation example. The refreshed cache parent also rejects replacements that expire during preparation or retry.
Runtime behavior: lost work-item ownership cancels cooperative work sooner; expired acquisition sets return null or throw from AcquireAsync. Queue-only renewal errors remain logged. EmptyLock provides no exclusive protection, cancellation is not fencing, and provider timeouts remain necessary.
Validation at
3a888094addd3c8c0abefec38668dd7aa7a9e845: PR Build and push Build passed: 2,369 passed / 22 skipped / 0 failed. Local Release build, 194 conditional-cache cases, 67 lock/worker cases, formatting, and fresh binary APICompat passed. Existing ASPIRE010 warning remains.Before merge/release: the child corrections must actually be incorporated into #580 before its dependent review findings can be closed. Combined/provider validation must be refreshed after the October 27 cache correction; older Redis results are historical. Durable provider registrations, non-Redis runtime limitations, next-release and documented performance/runtime acceptance remain separate requirements. No merge or release approval is inferred from green CI.
Verification and implementation details
Exact parent #580:
64a4fa6a68e42beff1d583c6763bc29035bf690a, including #5700aa9d6804beefe88401eb7eddc0466afdc6d85ad. Mainbe10785cfed4e1850350dfaa024e1c3f25b7fac7is an ancestor. The signed forward merge preserves both adjacent skill notes: immediate loss cancellation/acquisition latency and expired replacement rejection. Production code and tests merged cleanly.Fresh binary API comparison: Microsoft.DotNet.ApiCompat 10.0.402 compared assemblies built from exact main
be10785cagainst this final candidate. Foundatio net8.0/net10.0 and Foundatio.TestHarness net10.0 all exited 0 with no breaking changes, parameter-name checks enabled, and no suppressions. Binary compatibility is distinct from the intentional runtime behavior above.Current local verification: Release build passed with zero errors and one existing ASPIRE010 warning. Conditional-cache selection: 194 passed; InMemoryLockTests plus WorkItemJobTests: 67 passed, no skips/failures. Changed-file dotnet format verification and git diff --check passed. Both full hosted suites executed 2,391 cases: 2,369 passed, 22 existing skips, zero failures.
The focused XML confirms these child regressions passed:
Historical boundary: October 23 combined verification at
70533664included old #586fffdf32f: core 2,484 passed/22 skipped, Redis 778 passed/18 skipped, and APICompat clean. All 30 lock and 12 added cache cases executed against matching binaries, using isolated Redis registrations. Those results are not relabeled as current provider proof after the October 27 correction. Durable provider-main/CI registration and non-Redis runtime coverage were not established by that run. The earlier all-provider source build also required an isolated RabbitMQ sample dependency overlay; raw unmodified provider-main success is not claimed. No combined stack or benchmark was rerun in this refresh.Normal push CI publishes preview packages to GitHub Packages/Feedz; release publishing requires a version tag and was skipped. No workflow or security settings changed.