Skip to content

Fix Ollama keyless provider requests#240

Open
pollychen-lab wants to merge 1 commit into
404-PF:mainfrom
pollychen-lab:fix/issue-234-ollama-api-key
Open

Fix Ollama keyless provider requests#240
pollychen-lab wants to merge 1 commit into
404-PF:mainfrom
pollychen-lab:fix/issue-234-ollama-api-key

Conversation

@pollychen-lab

@pollychen-lab pollychen-lab commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Summary

  • omit the Authorization header for OpenAI-compatible requests when the resolved API key is blank
  • share the same header builder across complete, streaming, and model fetch requests
  • add regression tests for keyless complete and streaming requests

Closes #234.

Validation

  • npm run build --prefix /tmp/commit-echo-issue-234-verify
  • node --test /tmp/commit-echo-issue-234-verify/tests/providers/provider-adapters.test.mjs
  • npx --prefix /tmp/commit-echo-issue-234-verify prettier --check /tmp/commit-echo-issue-234-verify/src/providers/openai-compatible.ts
  • git diff --check

Summary by CodeRabbit

  • Bug Fixes

    • Improved compatibility with providers that do not require an API key.
    • Requests now omit the authorization header when no API key is configured.
    • Streaming requests correctly handle empty completion responses.
  • Tests

    • Added coverage for keyless standard and streaming requests.

@coderabbitai

coderabbitai Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: bd2fa577-283c-4ae6-828b-aa309cba9155

📥 Commits

Reviewing files that changed from the base of the PR and between 1910b09 and f28babf.

📒 Files selected for processing (2)
  • src/providers/openai-compatible.ts
  • tests/providers/provider-adapters.test.mjs
📜 Recent review details
🧰 Additional context used
📓 Path-based instructions (3)
src/**/*.ts

📄 CodeRabbit inference engine (AGENTS.md)

The project is ESM-only: use .js extensions in local imports and keep verbatimModuleSyntax-compatible TypeScript imports (for example, import type for type-only imports).

Files:

  • src/providers/openai-compatible.ts
src/providers/**/*.ts

📄 CodeRabbit inference engine (AGENTS.md)

All LLM provider implementations must conform to the shared Provider interface and integrate with the provider abstraction used by the factory/registry system.

Files:

  • src/providers/openai-compatible.ts
tests/**/*.mjs

📄 CodeRabbit inference engine (AGENTS.md)

tests/**/*.mjs: Use Node.js built-in node:test for test files in tests/ and tests/e2e/; do not use Jest or Mocha.
Write test assertions with node:assert/strict in test files under tests/ and tests/e2e/.

Files:

  • tests/providers/provider-adapters.test.mjs
🔇 Additional comments (3)
tests/providers/provider-adapters.test.mjs (2)

345-370: LGTM!


461-486: LGTM!

src/providers/openai-compatible.ts (1)

22-34: 🩺 Stability & Availability

No undefined guard needed for apiKey here. ChatParams.apiKey is required, and internal callers pass '' when the key is absent, so apiKey.trim() won’t see undefined.

			> Likely an incorrect or invalid review comment.

📝 Walkthrough

Walkthrough

OpenAI-compatible provider requests now share header construction that omits authorization for empty API keys. Completion, streaming, and model-fetching paths use the helper, with tests covering keyless completion and streaming behavior.

Changes

OpenAI-compatible headers

Layer / File(s) Summary
Conditional headers and keyless request tests
src/providers/openai-compatible.ts, tests/providers/provider-adapters.test.mjs
A shared helper trims API keys and conditionally adds the Bearer authorization header for completion, streaming, and model requests. Tests verify authorization is omitted for empty keys and that an ending-only stream yields no chunks.

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

Possibly related PRs

  • 404-PF/commit-echo#239: Updates the same provider request paths and adds matching keyless authorization tests.

Suggested reviewers: 404-page-found

Poem

I’m a rabbit with headers neat,
No empty key gets sent to meet.
Streams stay quiet at [DONE],
Models and completions share the run.
Hop, hop—clean requests are spun!

🚥 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 is concise and accurately reflects the keyless provider request fix in this PR.
Linked Issues check ✅ Passed The PR omits Authorization when the API key is blank, which addresses the keyless Ollama requirement in #234.
Out of Scope Changes check ✅ Passed The changes stay focused on shared header handling and regression tests for the provider keyless flow.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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.

@404-Page-Found

Copy link
Copy Markdown
Contributor

@pollychen-lab merge state dirty

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.

Ollama provider requires OLLAMA_API_KEY despite needingApiKey: false

2 participants