Skip to content

fix(sync): reject async strategy callbacks - #307

Merged
thomhurst merged 16 commits into
mainfrom
issue-225-sync-execution
Aug 25, 2026
Merged

fix(sync): reject async strategy callbacks#307
thomhurst merged 16 commits into
mainfrom
issue-225-sync-execution

Conversation

@thomhurst

Copy link
Copy Markdown
Owner

Summary

  • keep queued rate-limit waits synchronous under Execute, including non-pumping SynchronizationContext and net48 coverage
  • reject built-in async callbacks/generators before synchronous Execute, ExecuteOutcome, or ExecuteWithContext; add TimeoutGeneratorSync and typed/untyped BreakDurationGeneratorSync
  • dispatch synchronous circuit-monitor transitions with async observers on the thread pool
  • add KEV012, allocation/benchmark coverage, execution compatibility docs, and refreshed API metadata

Test plan

  • dotnet build Kevlar.slnx -c Release
  • core tests: net8.0 (1042), net10.0 (1067)
  • integration tests (151), analyzer tests (89), rate-limiter adapter tests (29)
  • allocation gates: net8.0 + net10.0 (4 each; sync built-ins/composition 0 B/op)
  • net48 compatibility and netstandard2.1 compatibility
  • pwsh scripts/Build-ApiDocs.ps1 and pwsh scripts/Verify-ApiDocs.ps1
  • npm ci and npm run build in docs

Benchmark

BenchmarkDotNet, .NET 10, five-strategy synchronous happy path (RateLimit -> Timeout -> Retry -> CircuitBreaker -> ConcurrencyLimit):

Build Mean Allocated
before (c267219) 288.2 ns 0 B
current 286.8 ns 0 B
Polly comparison 544.6 ns 88 B

Closes #225

@coderabbitai

coderabbitai Bot commented Aug 25, 2026

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: a7f52525-99b7-4cbc-9745-99e319b8e8a2


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f43787de2c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/Kevlar/Strategy.cs Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 341c32f7f4

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/Kevlar/Strategies/CircuitBreaker/CircuitBreakerCore.cs Outdated
@thomhurst

Copy link
Copy Markdown
Owner Author

Resolved the suppressed ExecutionContext-flow deadlock in f76f20d. Validation: Release build; core tests net8 1,044/1,044 and net10 1,069/1,069.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f76f20d3d8

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/Kevlar.Analyzers/PipelineHazardAnalyzer.cs Outdated
@thomhurst
thomhurst force-pushed the issue-225-sync-execution branch from f76f20d to c70d4a9 Compare August 25, 2026 09:51

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c70d4a9d80

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/Kevlar.Analyzers/PipelineHazardAnalyzer.cs Outdated
@thomhurst
thomhurst force-pushed the issue-225-sync-execution branch from c70d4a9 to b9e05f7 Compare August 25, 2026 10:15

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b9e05f7e90

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/Kevlar/Strategies/Retry/RetryStrategy.cs Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8b14263ecd

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/Kevlar.Extensions.RateLimiting/RateLimiterStrategy.cs Outdated
@thomhurst
thomhurst force-pushed the issue-225-sync-execution branch from 8b14263 to 72b0171 Compare August 25, 2026 10:44

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 72b01718bd

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/Kevlar/Strategies/Timeout/TimeoutOptions.cs
Comment thread src/Kevlar.Analyzers/PipelineHazardAnalyzer.cs Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 98292b6b1c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/Kevlar.Analyzers/PipelineHazardAnalyzer.cs
@thomhurst
thomhurst force-pushed the issue-225-sync-execution branch from 98292b6 to 20004a6 Compare August 25, 2026 11:19

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 20004a6839

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/Kevlar/Strategies/Fallback/FallbackStrategy.cs Outdated
Comment thread src/Kevlar.Analyzers/PipelineHazardAnalyzer.cs Outdated
@thomhurst

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3a5b26d431

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/Kevlar/Strategies/Fallback/FallbackStrategy.cs Outdated
Comment thread src/Kevlar.Chaos/Internal/BehaviorChaosStrategy.cs Outdated
Comment thread src/Kevlar.Analyzers/PipelineHazardAnalyzer.cs Outdated
@thomhurst

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3751264487

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/Kevlar.Extensions.RateLimiting/ShieldRateLimiterExtensions.cs Outdated
Comment thread src/Kevlar.Analyzers/PipelineHazardAnalyzer.cs Outdated
@thomhurst

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1de4f801ad

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/Kevlar.Analyzers/PipelineHazardAnalyzer.cs
@thomhurst

Copy link
Copy Markdown
Owner Author

Allocation gate repair: .NET 8.0.30/Linux reports the same 616 B/op typed-hedge pool-miss path on unchanged main. Raised the budget to 624 B/op, preserving an 8-byte margin while still rejecting the +24-byte boxing regression at 640 B/op. Exact Linux net8/net10 allocation gates pass. @codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ceda1677d9

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread tests/Kevlar.AllocationTests/AllocationBudgetTests.cs Outdated
@thomhurst

Copy link
Copy Markdown
Owner Author

Rebased onto current main and preserved enriched telemetry across sync-execution guards. Validation: Release build; Kevlar.Tests net8 (1102) and net10 (1127); AllocationTests 4/4 both; analyzer tests 91; rate-limiter adapter tests 31; chaos tests 39. @codex review

@thomhurst
thomhurst force-pushed the issue-225-sync-execution branch from ceda167 to aac9047 Compare August 25, 2026 12:27
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Something went wrong. Try again later by commenting “@codex review”.

Unknown error
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: aac90471f3

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/Kevlar/ShieldExtensions.cs Outdated
Comment thread src/Kevlar.Analyzers/PipelineHazardAnalyzer.cs Outdated
@thomhurst
thomhurst force-pushed the issue-225-sync-execution branch from aac9047 to 7d5797f Compare August 25, 2026 12:41
@thomhurst

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7d5797f1ce

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/Kevlar.Analyzers/PipelineHazardAnalyzer.cs
@thomhurst

Copy link
Copy Markdown
Owner Author

@codex review

@thomhurst
thomhurst force-pushed the issue-225-sync-execution branch from 7d5797f to 0ab8dac Compare August 25, 2026 12:48

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0ab8dac805

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/Kevlar.Analyzers/PipelineHazardAnalyzer.cs
@thomhurst
thomhurst force-pushed the issue-225-sync-execution branch from 0ab8dac to d7415cc Compare August 25, 2026 13:05
@thomhurst

Copy link
Copy Markdown
Owner Author

Resolved both remaining KEV012 review findings and rebased onto current main.

Validation:

  • dotnet build Kevlar.slnx -c Release
  • analyzer tests: 93/93
  • documentation verification
  • Docusaurus production build

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d7415ccd3f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/Kevlar.Analyzers/PipelineHazardAnalyzer.cs Outdated
@thomhurst

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Nice work!

Reviewed commit: 92f56a49b9

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@thomhurst
thomhurst merged commit 6194c18 into main Aug 25, 2026
10 checks passed
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.

fix(sync): queued rate-limit waits go async under sync Execute; sync generator twins; analyzer for async callbacks with Execute

1 participant