Skip to content

Reject a negative TimeSpan timeout - #708

Merged
kblok merged 1 commit into
mainfrom
fix-negative-timespan-timeout
Oct 21, 2026
Merged

kblok merged 1 commit into
mainfrom
fix-negative-timespan-timeout

Conversation

@kblok

@kblok kblok commented Oct 21, 2026 •

Copy link
Copy Markdown
Member

A negative TimeSpan such as TimeSpan.FromTicks(-1) rounded up to 0 ms, passed the check, and scheduled an immediate timeout. It now throws ArgumentOutOfRangeException, like a negative TotalMs. The conversion is .NET-only (upstream takes milliseconds), so the regression tests carry no upstream link. This follows the review comment that arrived on #704 after it merged.

🤖 Generated with Claude Code

https://claude.ai/code/session_01TQmtUKfZTs497KkiGSfYk9

Summary by CodeRabbit

  • Bug Fixes
    • Infinite timeout values are now correctly interpreted as no timeout.
    • Invalid negative timeout values are rejected.
    • Positive sub-millisecond timeout values are rounded up to the nearest millisecond.

@kblok kblok added the ready-to-tag Ready to tag a release label Oct 21, 2026
@coderabbitai

coderabbitai Bot commented Oct 21, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: hardkoded/vercel-ai-sdk-dotnet/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 4361bef6-817e-45eb-b805-d55e9b8195ac
📥 Commits

Reviewing files that changed from the base of the PR and between 74478bd and 173f407.

📒 Files selected for processing: 2
  • src/Vercel.AI/Prompt/TimeoutConfiguration.cs
  • tests/Vercel.AI.Tests/TimeoutConfigurationTests.cs
🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

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

📝 Walkthrough

The TimeSpan conversion now treats Timeout.InfiniteTimeSpan as unlimited, rejects other negative values, and retains upward rounding for nonnegative durations. Tests cover these behaviors.

Changes: Timeout conversion

Layer / File(s) Summary
Timeout conversion and validation
src/Vercel.AI/Prompt/TimeoutConfiguration.cs, tests/Vercel.AI.Tests/TimeoutConfigurationTests.cs
The conversion handles infinite timeouts before validation and throws ArgumentOutOfRangeException for other negative durations. Oversized nonnegative durations remain unlimited. Tests cover negative and infinite values, and upward rounding of a positive sub-millisecond duration.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Possibly related issues

  • Issue 697 in hardkoded/vercel-ai-sdk-dotnet — Its description concerns timeout configuration for generateText and streamText. This change updates TimeSpan timeout behavior.
Merge Risk: ⚪ Minimal · up to 173f4

The timeout change appears mergeable after normal checks; no outstanding behavior issue is identified.

Security architecture risk: ⚪ Minimal · up to 173f4

The change rejects invalid negative durations before timeout setup. Existing unlimited-timeout behavior is preserved, with no identified expansion of access or authority.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The demonstrated exposure is limited to callers supplying TimeSpan timeout values. The scoped comparison adds no production entrypoint or privilege transition; public declarations in the new test file are test code, not additional service endpoints.

Trust Boundaries and Controls

  • observed — The conversion checks the infinite sentinel first, rejects other negative inputs, and then returns a newly constructed configuration. Rejected values do not reach timer creation through this conversion, and the changed code introduces no shared-state mutation or partial transition requiring cleanup.
Pre-merge checks: Passed 5
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: rejecting negative TimeSpan timeout values.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Upstream Parity Passed The PR description names only port-repository pull requests (#708 and #704) and states that the conversion is .NET-only because upstream accepts milliseconds. It does not name a vercel/ai commit, upst…
Full details: Upstream Parity

Explanation

The PR description names only port-repository pull requests (#708 and #704) and states that the conversion is .NET-only because upstream accepts milliseconds. It does not name a vercel/ai commit, upstream pull request, or upstream file. Under the applicability rule, the Upstream parity check does not apply.

✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Usage-based review receipt

  • Mode: Continue automatically
  • Reviewed files: 2
  • Waived: $0.50 (charged $0.00)
  • View usage details

Note

This review exceeded your plan’s limits and used usage-based reviews—free during trial, billed after paid activation unless disabled. Manage usage-based reviews.

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

@kblok
kblok merged commit 4144b33 into main Oct 21, 2026
2 checks passed
@kblok
kblok deleted the fix-negative-timespan-timeout branch October 21, 2026 19:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-tag Ready to tag a release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant