Harden Kevlar shield boundaries

[thomhurst]

Summary

• translate the migrated CLI circuit breaker to five consecutive failures, so successes reset the streak
• saturate extreme module retry delays at TimeSpan.MaxValue and preserve zero delay
• correct Kevlar migration/lifecycle docs and add focused regressions

Validation

• ResilientCliCommandExecutorTests: 2 passed
• ModuleConfigurationTests: 35 passed
• OptionsGenerator suite: 817 passed; one unrelated PID-fixture failure tracked in Stabilize long-running child timeout fixture startup #3995
• ModularPipelines.slnx Release build: 0 warnings/errors
• OptionsGenerator Release build: 0 warnings/errors
• targeted format verification: clean
• Docusaurus production build: 326 documents

Fixes #4001
Refs #3980

Summary by CodeRabbit

• Bug Fixes

  • Improved retry-delay handling for zero and maximum-duration values, preventing invalid timing results.
  • Refined resilience behavior so circuit breakers correctly track consecutive command failures while retries operate as expected.
  • Improved asynchronous command execution reliability.

• Documentation

  • Clarified that module execution uses timeout handling and the configured resilience strategy, including retries and other safeguards.
  • Updated migration guidance with the required retry-count parameter.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

[coderabbitai]

Warning

Review limit reached

Next included review available in 43 minutes.

View limit details

Limit details: You’ve used all 4 included reviews currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 7dde3c5f-767f-46f1-8388-52627d4f112a

📥 Commits

Reviewing files that changed from the base of the PR and between ed047cc and deb4040.

📒 Files selected for processing (2)

• tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator.Tests/TypeDetection/ResilientCliCommandExecutorTests.cs
• tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator/TypeDetection/ResilientCliCommandExecutor.cs

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 525921d8-54c4-4aa6-94b1-caf6bab584fb

📥 Commits

Reviewing files that changed from the base of the PR and between 9c23b33 and ed047cc.

📒 Files selected for processing (6)

• docs/docs/architecture/module-execution-lifecycle.md
• docs/docs/migrating-to-v3.md
• src/ModularPipelines/Configuration/ModuleRetryShieldFactory.cs
• test/ModularPipelines.UnitTests/Configuration/ModuleConfigurationTests.cs
• tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator.Tests/TypeDetection/ResilientCliCommandExecutorTests.cs
• tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator/TypeDetection/ResilientCliCommandExecutor.cs

Included review availability: Your plan provides up to 4 included reviews per hour; 0 remain after this review.

---

📝 Walkthrough

Walkthrough

The change hardens retry-delay calculations, updates resilient CLI policy ordering and asynchronous execution, adds circuit-breaker and boundary tests, and corrects lifecycle and migration documentation.

Changes

Resilience hardening

Layer / File(s)	Summary
Retry-delay boundary handling src/ModularPipelines/Configuration/ModuleRetryShieldFactory.cs, test/ModularPipelines.UnitTests/Configuration/ModuleConfigurationTests.cs	Zero delays return TimeSpan.Zero. Saturated delays return TimeSpan.MaxValue. Tests cover both boundaries.
CLI resilience policy behavior tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator/TypeDetection/ResilientCliCommandExecutor.cs, tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator.Tests/TypeDetection/ResilientCliCommandExecutorTests.cs	The circuit breaker runs before retries. Async calls use ConfigureAwait(false). Tests cover interleaved successes, five consecutive failures, and retry-attempt counting.
Lifecycle and migration documentation docs/docs/architecture/module-execution-lifecycle.md, docs/docs/migrating-to-v3.md	Documentation describes the resilience shield and includes the retry count in WithRetry(count, ...) examples.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to ed047

The PR adjusts retry and circuit-breaker boundary behavior, adds focused regressions, and updates documentation; no actionable merge-blocking risk remains after normal checks and review.

Poem

A rabbit checks each retry hop,
Keeps zero delays at zero’s stop.
Five failures make the circuit close,
Success resets the counted woes.
Docs now name each shield and count—
Neat paths through the burrow mount.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the resilience boundary hardening implemented by the pull request.
Linked Issues check	✅ Passed	The changes address all requirements in issue #4001, including circuit behavior, delay saturation, documentation, and regression tests.
Out of Scope Changes check	✅ Passed	All implementation, documentation, and test changes directly support the objectives in issue #4001.
Docstring Coverage	✅ Passed	Docstring check was indeterminate for this PR — some files could not be analyzed in time. Not blocking.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch issue-4001-kevlar-boundaries

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[greptile-apps]

Greptile Summary

The PR hardens resilience boundaries in module retry delays and generated CLI command execution.

• Preserves zero retry delays and saturates extreme exponential delays at TimeSpan.MaxValue.
• Configures the CLI circuit breaker to track five consecutive failed commands, with retries composed inside that boundary.
• Adds focused regression coverage and corrects lifecycle and migration documentation.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename	Overview
src/ModularPipelines/Configuration/ModuleRetryShieldFactory.cs	Adds explicit zero-delay preservation and saturation before converting extreme floating-point tick values to TimeSpan.
tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator/TypeDetection/ResilientCliCommandExecutor.cs	Replaces ratio-based circuit breaking with consecutive-failure tracking and reorders resilience composition so retries contribute one final command outcome.
tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator.Tests/TypeDetection/ResilientCliCommandExecutorTests.cs	Adds regressions for interleaved successes, five consecutive failures, and command-level rather than attempt-level breaker accounting.
test/ModularPipelines.UnitTests/Configuration/ModuleConfigurationTests.cs	Adds boundary tests for zero-delay and maximum-delay retry calculations.
docs/docs/architecture/module-execution-lifecycle.md	Clarifies that module execution uses timeout handling and the configured resilience shield.
docs/docs/migrating-to-v3.md	Corrects retry migration examples to include the required count and advanced shield argument.

_{Reviews (7): Last reviewed commit: "fix(resilience): count failed commands" | Re-trigger Greptile}

[claude]

Review: Harden Kevlar shield boundaries

Solid, well-scoped hardening PR. Went through the diff, the ModuleRetryShieldFactory.CalculateDelay math (verified the TimeSpan.MaxValue saturation and the 0 * Infinity = NaN guard numerically), Kevlar's actual source/docs for ConsecutiveFailures semantics, and the new/updated tests.

What's good:

• ModuleRetryShieldFactory.CalculateDelay: the baseDelay == TimeSpan.Zero early-return and the jitteredTicks >= TimeSpan.MaxValue.Ticks saturation guard correctly close both an edge case that previously produced NaN (huge retryAttempt with zero baseDelay) and one that could overflow on cast to long (huge baseDelay/jitterFactor). New tests exercise both boundaries directly.
• Docs fixes (migrating-to-v3.md, module-execution-lifecycle.md) are accurate against the real WithRetry/WithShield signatures and the actual execution pipeline — no stale references left behind.
• ConfigureAwait(false) additions in ResilientCliCommandExecutor match existing conventions in sibling executors.

One behavioral concern worth a look — ResilientCliCommandExecutor.cs:

The circuit breaker is built as .Retry(...).CircuitBreaker(...), i.e. Retry is the outer strategy and CircuitBreaker is the inner one, so the breaker observes every individual retry attempt, not just the final outcome of a logical CLI invocation. That composition order predates this PR, but switching CircuitBreaker's config from ratio-based (FailureRatio/MinimumThroughput/SamplingWindow) to ConsecutiveFailures = circuitBreakerThreshold makes that distinction matter a lot more, because there's no longer a minimum-throughput/sampling-window gate slowing down how fast consecutive attempts accumulate.

With the production defaults (maxRetries: 3, circuitBreakerThreshold: 5), a single CLI invocation that exhausts all retries produces 4 consecutive failure signals to the breaker (1 initial attempt + 3 retries). So in practice the breaker opens after roughly 2 fully-failed CLI commands, not the "5 failures" implied by:

• the class's own XML doc comment: Circuit breaker (5 failures opens circuit for 30 seconds)
• the new test's name: CircuitBreaker_Opens_AfterFiveConsecutiveFailures

That test currently constructs the executor with maxRetries: 0, which makes attempts and logical calls equivalent and hides this gap — it doesn't exercise the production defaults where the discrepancy actually shows up.

Suggested follow-up (either works, but worth picking one deliberately rather than leaving it implicit):

1. Reorder the shield so CircuitBreaker wraps Retry (breaker outer, retry inner) if Kevlar's composition semantics allow it — then the breaker only sees the outcome after retries are exhausted, matching "N failed commands" as documented.
2. Or keep the current order but recalibrate, e.g. scale circuitBreakerThreshold relative to maxRetries (or document explicitly that the threshold counts attempts, not calls) — and add a test using the production defaults (maxRetries: 3) so the actual trip point is locked in and visible, rather than only testing the maxRetries: 0 case.

Not a regression introduced by this PR's composition mechanics (the ordering was already there), but the switch to consecutive-failure counting is what makes the gap between documented and actual behavior significant — worth deciding intentionally before this ships, since it changes how aggressively CLI resilience trips in production.

[thomhurst]

The macOS Restore and build solutions step was still running after about 70 minutes (the same step completed in 13 minutes on the adjacent PR). The run's cancel request also stalled, so empty commit b7f885a supersedes it and restarts checks.

[claude]

Review: Harden Kevlar shield boundaries (follow-up)

No new commits with code changes since my previous review — the only addition is the empty ci: restart stalled macOS job commit (b7f885a), which touches no files and was just to unstick a stalled macOS CI run. The diff is otherwise identical to what I already reviewed.

The one item I flagged previously is still open and worth a decision before merge:

ResilientCliCommandExecutor.cs — the circuit breaker wraps the retry policy (Retry outer, CircuitBreaker inner), so switching to ConsecutiveFailures-based tripping means the breaker counts individual retry attempts, not failed logical CLI invocations. With the production defaults (maxRetries: 3, circuitBreakerThreshold: 5), a single fully-exhausted CLI call already emits 4 consecutive failure signals, so the breaker trips after roughly 2 failed commands — not the "5 failures" implied by the class's XML doc and by the new test name CircuitBreaker_Opens_AfterFiveConsecutiveFailures (which only exercises maxRetries: 0, so attempts and calls coincide there and the gap doesn't show up).

This isn't a regression from this PR (the Retry/CircuitBreaker ordering predates it), but the switch to consecutive-failure counting is what makes the documented-vs-actual behavior gap meaningful. Suggest picking one deliberately:

1. Reorder so CircuitBreaker wraps Retry, so the breaker only sees post-retry outcomes, matching "N failed commands" as documented, or
2. Keep the current order but recalibrate circuitBreakerThreshold relative to maxRetries (or document that the threshold counts attempts), and add a test using production defaults (maxRetries: 3) so the actual trip point is locked in.

Everything else from the prior review still stands: the CalculateDelay zero-delay/TimeSpan.MaxValue saturation logic is correct and well-tested, and the docs fixes are accurate.

[thomhurst]

Addressed the circuit-breaker review in 99ebe70. Kevlar composition now places CircuitBreaker outside Retry, so the threshold counts failed logical CLI commands rather than individual attempts. Added a production-retry regression (3 retries, 5-command threshold). Validation: OptionsGenerator tests 819/819; Release build 0 warnings/errors.

[claude]

Review: Harden Kevlar shield boundaries (follow-up #2)

Re-reviewed at head commit 99ebe701. This resolves the one open item from my previous review.

Circuit breaker / retry ordering — confirmed fixed.

ResilientCliCommandExecutor.cs now builds the shield as .CircuitBreaker(...).Retry(...), i.e. the breaker is the outer strategy and retry is inner, so ConsecutiveFailures now counts failed logical CLI commands rather than individual retry attempts — matching both the class's XML doc (Circuit breaker (5 failures opens circuit for 30 seconds)) and the new test name.

The new CircuitBreaker_CountsFailedCommands_NotIndividualRetryAttempts test is the right regression here: it uses production-shaped defaults (maxRetries: 3, circuitBreakerThreshold: 5) rather than the maxRetries: 0 case that hid the original gap, and asserts inner.ExecutionCount == 20 (5 commands × 4 attempts each) while only the breaker trips at the 6th call. I traced through the composition and this checks out — with retry nested inside the breaker, each logical command's retries collapse into a single pass/fail signal before the breaker ever sees it.

Also independently re-verified ModuleRetryShieldFactory.CalculateDelay's two new guards:

• baseDelay == TimeSpan.Zero short-circuit avoids the 0 * Infinity = NaN case for huge retryAttempt.
• jitteredTicks >= TimeSpan.MaxValue.Ticks catches the case where TimeSpan.MaxValue.Ticks (a long) loses precision when implicitly widened to double (rounds up past long.MaxValue), which would otherwise overflow on the (long) cast. Both new tests (RetryDelayCalculator_ZeroDelay_RemainsZeroAtMaximumAttempt, RetryDelayCalculator_MaximumDelay_RemainsInTimeSpanRange) land on the exact boundary values these guards protect.

No new issues found. One pre-existing, non-blocking observation (not introduced by this PR, not worth blocking on): the ResilienceStatistics record at the bottom of ResilientCliCommandExecutor.cs is never constructed or referenced anywhere in the codebase — if it's not on a near-term roadmap for exposing metrics, it'd be worth deleting rather than carrying an unused public type.

Nice fix — reordering the shield instead of just recalibrating the threshold is the more robust choice, since it makes the breaker's semantics ("N failed commands") hold regardless of what maxRetries is configured to.

[thomhurst]

Rebased onto current main (13fd094). Validation: ModuleConfiguration 35/35; resilient CLI executor 3/3; core and OptionsGenerator Release builds both succeeded with 0 warnings/errors.

[claude]

Review: Harden Kevlar shield boundaries (follow-up on ab985da)

Correction to my own previous review: I mis-stated the pre-fix chain order in ResilientCliCommandExecutor.cs (I described it as .Retry(...).CircuitBreaker(...) when it was actually .CircuitBreaker(...).Retry(...)), and drew the wrong conclusion from it. I went back to Kevlar's own composition docs to get this right before commenting again.

The new commit (ab985da) swapped the strategy order the wrong way.

Kevlar's composition doc states the rule explicitly, with the exact shape used here as the worked example:

Shield
    .Timeout(...)
    .Retry(3)                            // 2. retries happen inside that budget
    .CircuitBreaker(consecutiveFailures: 5, ...)  // 3. breaker sees each attempt

"The first strategy in a chain is the outermost — the same rule as ASP.NET middleware."

ResilientCliCommandExecutor.cs (lines 59-95) now builds the shield as .Retry(...).CircuitBreaker(...) — Retry first (outer), CircuitBreaker second (inner). Per Kevlar's own docs, that's precisely the ordering where "breaker sees each attempt," not each logical CLI invocation. With production defaults (maxRetries: 3, circuitBreakerThreshold: 5), one fully-exhausted CLI command still emits 4 consecutive failure signals (1 initial + 3 retries), so the breaker still opens after ~2 failed commands — not the "5 failures" documented in the class's own XML comment (Circuit breaker (5 failures opens circuit for 30 seconds), still unchanged at the top of the file) or implied by the test name CircuitBreaker_Opens_AfterFiveConsecutiveFailures.

Before this commit, the order was .CircuitBreaker(...).Retry(...) — CircuitBreaker outer, Retry inner — which per the same doc rule means the breaker only observes the final outcome after Retry's internal loop finishes, i.e. one signal per logical CLI call. That was already the behavior the docs/tests describe; the swap in ab985da moved it away from that, not towards it.

Test coverage regressed along with it. The commit also deleted CircuitBreaker_CountsFailedCommands_NotIndividualRetryAttempts (the one test that exercised maxRetries: 3 together with the breaker) and hard-coded CreateExecutor to always use maxRetries: 0. With retries disabled, "attempts" and "logical calls" are the same thing, so no test in the suite can now distinguish the two, and this ordering regression won't be caught by CI.

Suggested fix: revert the ordering back to .CircuitBreaker(...).Retry(...) (breaker outer, retry inner) so the breaker counts failed commands as documented, and reinstate a test that builds the executor with production-like maxRetries (e.g. 3) alongside the circuit breaker so this distinction is locked in and visible in the suite — the deleted CircuitBreaker_CountsFailedCommands_NotIndividualRetryAttempts test was doing exactly that job.

Everything else from my prior reviews still stands unchanged: the CalculateDelay zero-delay/TimeSpan.MaxValue saturation logic is correct and well-tested, and the docs fixes are accurate.

[thomhurst]

Rebased onto current main after Kevlar 0.8 landed and force-pushed with an exact lease. Validation: ModuleConfigurationTests 35/35 and ResilientCliCommandExecutorTests 3/3 passed; lightweight core and OptionsGenerator Release builds both succeeded with 0 warnings/errors; diff checks clean. Verified remote head: ed047cc.

[claude]

Review: Harden Kevlar shield boundaries (re-review at ed047cc)

Re-reviewed at the current head commit (ed047cc8, after the "Kevlar 0.8" rebase). This addresses the concern from my prior review on this PR (the .Retry(...).CircuitBreaker(...) ordering regression introduced in ab985da).

Verified fixed:

• ResilientCliCommandExecutor.cs now builds the shield as .CircuitBreaker(...) (outer) then .Retry(...) (inner). Per Kevlar's own composition docs ("the first strategy in a chain is the outermost"), this means the breaker observes one signal per logical CLI invocation (after retries are exhausted), not one per individual retry attempt — matching the class's XML doc ("Circuit breaker (5 failures opens circuit for 30 seconds)") and the test name CircuitBreaker_Opens_AfterFiveConsecutiveFailures.
• The previously-deleted CircuitBreaker_CountsFailedCommands_NotIndividualRetryAttempts test is reinstated, using maxRetries: 3 alongside the breaker, and asserts inner.ExecutionCount == 20 (5 failed commands × 4 attempts each) while the breaker still only opens after the 5th command — this is exactly the regression-catching coverage I'd asked for, and I ran it locally: it passes.
• ModuleRetryShieldFactory.CalculateDelay still has both guards from earlier in the PR: the baseDelay == TimeSpan.Zero early return (avoids 0 * Infinity = NaN for large retry attempts) and the jitteredTicks >= TimeSpan.MaxValue.Ticks saturation clamp (avoids overflow on cast to long). Both are exercised by dedicated tests.
• Docs changes (migrating-to-v3.md, module-execution-lifecycle.md) are still accurate against current signatures.

No remaining correctness issues. Nice fix — the reordering plus the reinstated test closes the gap between documented and actual circuit-breaker behavior.

[thomhurst]

Rebased three commits onto Kevlar 0.9 main (#4032) and force-pushed with an exact lease. Validation: ModuleConfigurationTests 35/35; ResilientCliCommandExecutorTests 3/3; core and OptionsGenerator Release builds both 0 warnings/errors; diff check clean. Verified remote head: 69e709d.

[claude]

Review: Harden Kevlar shield boundaries (follow-up #3)

Re-reviewed at head commit 69e709d0 ("fix(resilience): count failed commands"), which lands on top of the Kevlar 0.8.0 → 0.9.0 bump.

Circuit breaker / retry ordering — confirmed correct again.

This commit re-applies the fix from my earlier follow-up review: ResilientCliCommandExecutor.cs now builds the shield as .CircuitBreaker(options => options.ConsecutiveFailures = circuitBreakerThreshold, ...).Retry(...) — breaker outer, retry inner — so ConsecutiveFailures counts failed logical CLI commands, not individual retry attempts, matching the class's XML doc ("Circuit breaker (5 failures opens circuit for 30 seconds)").

The reinstated CircuitBreaker_CountsFailedCommands_NotIndividualRetryAttempts test is doing the right job here (it's the same test that was dropped in the earlier ab985da regression): it exercises maxRetries: 3 alongside the breaker, drives 20 consecutive inner failures, and asserts inner.ExecutionCount == 20 (5 commands × 4 attempts each) while the 6th call is rejected with the circuit-open exit code without reaching _inner. I traced the composition and this checks out — with retry nested inside the breaker, each logical command's retries collapse into a single pass/fail signal before the breaker observes it, so this test would fail again if the ordering regressed a third time.

The Directory.Packages.props bump to Kevlar 0.9.0 and the corresponding switch from the ratio-based (FailureRatio/MinimumThroughput/SamplingWindow) circuit breaker config to ConsecutiveFailures looks like a deliberate, well-scoped migration to the new API rather than a side effect — consistent with the PR description.

No new issues found. Everything from my prior reviews still stands: the CalculateDelay zero-delay/TimeSpan.MaxValue saturation guards are correct and tested, and the docs fixes are accurate. The pre-existing, non-blocking observation about the unused ResilienceStatistics record at the bottom of the file is still there if it's worth cleaning up, but it's not introduced by this PR and not worth blocking on.

[thomhurst]

Rebased onto current main and resolved the add/add test conflict by preserving both main's tool-specific availability fixture and this PR's resilience fixtures. Dropped the obsolete empty CI-restart commit. Validation: ResilientCliCommandExecutorTests 4/4; ModuleConfigurationTests 35/35; core and OptionsGenerator Release builds 0 warnings/errors; scoped format and diff checks clean. Verified remote head: deb4040.

[claude]

Claude Code Review

Reviewed this PR's changes to the Kevlar resilience shield boundaries (ModuleRetryShieldFactory.cs, ResilientCliCommandExecutor.cs), plus the new tests and doc updates.

What this PR does:

• Reorders .CircuitBreaker() before .Retry() in the Kevlar Shield builder chain used by ResilientCliCommandExecutor.
• Switches the circuit breaker from a FailureRatio/MinimumThroughput/sampling-window design to ConsecutiveFailures.
• Adds ConfigureAwait(false).
• Clamps ModuleRetryShieldFactory.CalculateDelay for zero/TimeSpan.MaxValue edge cases.
• Adds corresponding tests and two doc wording fixes.

Verification performed:

• Confirmed against the upstream Kevlar source (Shield.BuildChain) that the first-added strategy is outermost — so .CircuitBreaker().Retry() means the circuit breaker only observes the final per-call outcome after all retries complete, correctly fixing the prior bug where individual retry attempts were each miscounted as separate circuit-breaker failures. This is the correct architectural fix rather than a workaround: it aligns the shield's failure counting with the logical operation (the whole retried call) instead of its physical attempts.
• Ran the new/updated tests (ResilientCliCommandExecutorTests and ModuleConfigurationTests.RetryDelayCalculator_*) — all pass and match the intended semantics.
• Checked for other call sites and leftover references to the removed FailureRatio/MinimumThroughput/SamplingWindow options — none found; OptionsGeneratorCommand.cs uses the unaffected default constructor overload.
• Confirmed the updated docs (docs/docs/migrating-to-v3.md, docs/docs/architecture/module-execution-lifecycle.md) match the actual WithRetry/Advanced.WithShield signatures.
• Verified CalculateDelay's zero-delay short-circuit correctly avoids a 0 * Infinity = NaN case with baseDelay = TimeSpan.Zero and a large retry attempt.

No correctness, design, or maintainability issues found. This is a solid, well-scoped fix — the shield ordering change addresses a real semantic bug (retries being double-counted as circuit-breaker failures) rather than just papering over symptoms, and it's backed by tests that exercise the actual composed behavior.

🤖 Generated with Claude Code