Skip to content

fix(retry): reject negative retry counts - #4495

Closed
sylvesterkaczmarek wants to merge 2 commits into
openai:mainfrom
sylvesterkaczmarek:fix/retry-negative-count-validation
Closed

fix(retry): reject negative retry counts#4495
sylvesterkaczmarek wants to merge 2 commits into
openai:mainfrom
sylvesterkaczmarek:fix/retry-negative-count-validation

Conversation

@sylvesterkaczmarek

Copy link
Copy Markdown
Contributor

Summary

  • constrain ModelRetrySettings.max_retries to non-negative values
  • preserve 0 as the explicit no-retry configuration
  • fail during settings validation instead of silently treating negative counts as an exhausted retry budget

The setting represents retries allowed after the initial request, so negative values are not meaningful. The backoff settings already use Pydantic numeric constraints; this applies the same validation boundary to the retry count.

Test plan

  • added focused regression coverage in tests/test_retry_count_validation.py
  • GitHub Actions

Issue number

N/A

@sylvesterkaczmarek
sylvesterkaczmarek marked this pull request as ready for review August 17, 2026 20:42
@seratch

seratch commented Aug 17, 2026

Copy link
Copy Markdown
Member

Thanks for the contribution. I confirmed that ModelRetrySettings(max_retries=-1) is currently accepted and that this patch rejects it. However, the existing runtime already treats every negative value as the same no-retry behavior as max_retries=0 across the runner and provider retry paths. The PR does not link a user report or demonstrate an observable consequence beyond accepting a semantically invalid configuration. This also differs from #3270, where negative backoff values produced invalid delays and immediate retries.

I am going to close this PR for now. If we see a concrete supported scenario where negative retry counts cause meaningful behavior beyond disabling retries, we can reconsider a compatibility-preserving validation change.

@seratch seratch closed this Aug 17, 2026
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.

2 participants