Add configurable timeout support for OpenAI client

[automaticgiant]

Summary

• allow configuring the OpenAI/Ollama timeout via CLI flag, environment variable, or settings without changing the 120s default
• thread the optional timeout through core Config so the OpenAI content generator respects it
• document the new knobs and add coverage for the precedence rules

Testing

• npx vitest run packages/core/src/config/config.test.ts
• npx vitest run packages/cli/src/config/config.test.ts

Summary by CodeRabbit

• New Features

  • Added timeout configuration for OpenAI/Ollama requests with three configuration methods: settings file (ollama.timeoutMs), environment variables (OPENAI_TIMEOUT_MS / OLLAMA_TIMEOUT_MS), and CLI flag (--openai-timeout-ms). Priority order: CLI flag → environment variables → settings (defaults to 120000ms).

• Documentation

  • Updated configuration documentation with timeout options and usage examples.

[coderabbitai]

Walkthrough

This pull request introduces comprehensive timeout configuration support for OpenAI/Ollama requests across multiple configuration layers. Users can now specify request timeouts via settings.json, environment variables (OPENAI_TIMEOUT_MS, OLLAMA_TIMEOUT_MS), or CLI arguments (--openai-timeout-ms), with a precedence order of CLI > environment > settings. The resolved timeout value is threaded through the configuration hierarchy into the core Config class.

Changes

Cohort / File(s)	Summary
Documentation Updates docs/cli/configuration.md	Documents new timeout configuration options: settings.json newollama.timeoutMs property, OPENAI_TIMEOUT_MS and OLLAMA_TIMEOUT_MS environment variables, and --openai-timeout-ms CLI argument with usage examples.
CLI Configuration Parsing packages/cli/src/config/config.ts	Adds parseTimeoutMs() utility for timeout validation, introduces --openai-timeout-ms CLI argument, implements precedence-based timeout resolution (CLI > environment > settings), and passes resolved timeout to Config constructor via contentGeneratorTimeoutMs property.
CLI Configuration Models packages/cli/src/config/settings.ts	Adds optional timeoutMs field to OllamaConfig interface and implements logic to map it to OPENAI_TIMEOUT_MS when environment variables are not already set.
CLI Configuration Tests packages/cli/src/config/config.test.ts	Extends MockConfig with private timeoutMs field and public getContentGeneratorTimeoutMs() getter; adds test cases verifying timeout precedence (CLI flag overrides env var and settings, env var overrides settings, settings fallback).
Core Configuration packages/core/src/config/config.ts	Adds optional contentGeneratorTimeoutMs parameter to ConfigParameters interface, stores it as private field in Config class, and propagates it to contentGeneratorConfig.timeout during auth refresh.
Core Configuration Tests packages/core/src/config/config.test.ts	Adds test case verifying that contentGeneratorTimeoutMs from Config parameters is correctly applied to content generator config timeout during auth refresh.

Sequence Diagram

sequenceDiagram
    participant CLI as CLI Arguments
    participant ENV as Environment Vars
    participant Settings as settings.json
    participant Parser as Timeout Parser
    participant Config as Config Class
    participant ContentGen as ContentGeneratorConfig

    CLI->>Parser: openai-timeout-ms value
    ENV->>Parser: OPENAI_TIMEOUT_MS / OLLAMA_TIMEOUT_MS
    Settings->>Parser: timeoutMs from OllamaConfig
    
    rect rgb(200, 220, 240)
        Note over Parser: Precedence: CLI > ENV > Settings
        Parser->>Config: contentGeneratorTimeoutMs (resolved)
    end
    
    Config->>Config: refreshAuth()
    alt contentGeneratorTimeoutMs defined
        Config->>ContentGen: Apply timeout
    end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• Precedence logic verification: Confirm the timeout resolution order (CLI > environment > settings) is correctly implemented in config.ts and that all three sources are properly parsed.
• Parameter threading: Verify contentGeneratorTimeoutMs flows correctly from CLI parsing through Config constructor to refreshAuth() and ultimately to contentGeneratorConfig.timeout.
• Test coverage: Ensure all timeout precedence scenarios are covered in both CLI and core config tests, particularly the fallback cases.
• Environment variable naming: Confirm both OPENAI_TIMEOUT_MS and OLLAMA_TIMEOUT_MS are consistently handled, with clear precedence rules when both are defined.

Poem

🐰 A rabbit's tale of timeouts refined,
Through layers of config, settings aligned,
CLI, env, and JSON too,
Precedence whispers what to do,
Now OpenAI waits just the right time! ⏱️✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.
Description check	❓ Inconclusive	The PR description lacks required template sections (Dive Deeper, Reviewer Test Plan, Testing Matrix, Linked issues), but covers the essential TLDR with clear change summary and testing commands.	Complete the description by adding Dive Deeper section with implementation details, Reviewer Test Plan with validation steps, Testing Matrix with platform coverage, and Linked issues section with any related issue references.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'Add configurable timeout support for OpenAI client' directly summarizes the main change—adding configurable timeout support for OpenAI/Ollama across CLI, environment variables, and settings.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 58bab62 and a6d9c81.

⛔ Files ignored due to path filters (1)

• package-lock.json is excluded by !**/package-lock.json

📒 Files selected for processing (6)

• docs/cli/configuration.md (3 hunks)
• packages/cli/src/config/config.test.ts (2 hunks)
• packages/cli/src/config/config.ts (5 hunks)
• packages/cli/src/config/settings.ts (2 hunks)
• packages/core/src/config/config.test.ts (1 hunks)
• packages/core/src/config/config.ts (4 hunks)

🧰 Additional context used

🧬 Code graph analysis (2)

packages/cli/src/config/config.test.ts (2)

packages/core/src/config/config.ts (1)

• ConfigParameters (109-158)

packages/cli/src/config/config.ts (1)

• loadCliConfig (270-449)

packages/core/src/config/config.test.ts (2)

packages/core/src/config/config.ts (1)

• Config (160-615)

packages/core/src/core/contentGenerator.ts (1)

• createContentGeneratorConfig (70-125)

🪛 LanguageTool

docs/cli/configuration.md

[style] ~218-~218: As an alternative to the over-used intensifier ‘very’, consider replacing this phrase.
Context: ...Ollama-compatible API calls. Useful for very large prompts or models that take a long time...

(EN_WEAK_ADJECTIVE)

🔇 Additional comments (17)

packages/cli/src/config/settings.ts (2)

61-61: LGTM! Clean interface addition.

The optional timeoutMs field is well-positioned in the OllamaConfig interface.

---

301-307: Timeout propagation logic looks correct.

The implementation properly respects environment variable precedence by only setting OPENAI_TIMEOUT_MS when both OPENAI_TIMEOUT_MS and OLLAMA_TIMEOUT_MS are undefined. The String() conversion is appropriate for environment variables.

packages/core/src/config/config.test.ts (1)

171-188: Excellent test coverage for timeout propagation.

This test effectively validates that contentGeneratorTimeoutMs is correctly applied to contentGeneratorConfig.timeout during auth refresh. The test follows established patterns and uses appropriate mocking.

docs/cli/configuration.md (3)

217-226: Clear documentation of the settings option.

The ollama.timeoutMs setting is well documented with appropriate defaults, use case description, and example. The 2-minute (120,000ms) default is reasonable for most scenarios while allowing override for edge cases.

---

285-287: Well-documented environment variable precedence.

The documentation clearly states that OPENAI_TIMEOUT_MS takes precedence over OLLAMA_TIMEOUT_MS when both are defined, which aligns with the implementation.

---

370-371: CLI flag documentation is concise and clear.

The --openai-timeout-ms flag documentation appropriately indicates it overrides both settings and environment variables, matching the intended precedence order.

packages/core/src/config/config.ts (4)

148-148: LGTM! Clean interface extension.

The optional contentGeneratorTimeoutMs parameter is appropriately added to ConfigParameters.

---

199-199: Field declaration follows conventions.

The private readonly modifiers are appropriate for this configuration value.

---

262-262: Straightforward constructor assignment.

---

303-305: Correct timeout application logic.

The !== undefined check is appropriate here as it allows explicit zero values while only applying the timeout when actually provided.

packages/cli/src/config/config.test.ts (2)

53-67: Well-structured mock extension for testing.

The MockConfig extension appropriately captures and exposes contentGeneratorTimeoutMs for test validation. The implementation follows existing patterns in the mock.

---

253-297: Comprehensive timeout precedence test coverage.

These three tests thoroughly validate the precedence order (CLI > environment > settings) with clear test cases:

1. CLI (86400000) overrides both env (60000) and settings (30000)
2. Environment (123456) overrides settings (789) when CLI absent
3. Settings (456789) used when neither CLI nor env provided

The tests are well-structured and provide excellent coverage of the configuration resolution logic.

packages/cli/src/config/config.ts (5)

38-57: Robust timeout parsing utility.

The parseTimeoutMs function properly handles edge cases:

• Returns undefined for null/undefined/empty string
• Validates finite and non-negative values
• Provides helpful warnings with source context
• Gracefully degrades by returning undefined on invalid input

The implementation correctly handles edge cases like NaN, Infinity, and negative values.

---

83-83: Appropriate type for CLI argument.

---

222-226: Clear CLI option definition.

The yargs option configuration is well-structured with an accurate description of override behavior.

---

360-381: Excellent timeout resolution implementation.

This implementation correctly establishes the precedence chain:

1. CLI flag (--openai-timeout-ms)
2. Environment variables (OPENAI_TIMEOUT_MS > OLLAMA_TIMEOUT_MS)
3. Settings (settings.ollama.timeoutMs)

Each source is validated through parseTimeoutMs, ensuring consistent error handling. The use of nullish coalescing (??) elegantly implements the fallback chain.

---

446-446: Timeout value properly threaded to core Config.

---

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 and usage tips.}

[automaticgiant]

yeah i think this is working. i had a 30min agentic loop earlier.

are we still maintaining this @tcsenpai?

are there other projects to merge with? was there any possibility of integrating back into gemini-cli or the qwen client you forked (after they forked gemini-cli)?

i'd like to collab on this as a daily driver, if you're interested.