feat: Add OpenRouter AI adapter (100+ models, free tier) - #4
Conversation
- New adapter: @superbiche/openrouter-paperclip-adapter - Supports 100+ models via OpenRouter API (DeepSeek, Gemini, Claude, GPT-4) - Free models available: DeepSeek Chat, Gemini Flash, Llama 3, Qwen - Automatic token tracking and cost estimation - Comprehensive error handling (auth, rate limits, timeouts) - Environment test for API connectivity validation - Full documentation with usage examples Built by @fatherkam for cost-effective multi-agent orchestration. Enables 50-70% cost savings vs Claude Max subscription.
📝 WalkthroughWalkthroughAdds a new ChangesOpenRouter AI Adapter Package
Sequence Diagram(s) sequenceDiagram
participant Adapter as Server execute()
participant Client as OpenRouterClient
participant OpenRouter as OpenRouter API
participant Caller as Adapter caller
Caller->>Adapter: invoke execute(ctx with prompt/config)
Adapter->>Client: chatCompletion(messages, options)
Client->>OpenRouter: POST /chat/completions (Authorization + timeout)
OpenRouter-->>Client: 200 OK (choices, usage)
Client-->>Adapter: OpenRouterResponse
Adapter->>Caller: AdapterExecutionResult (content, tokens, costUsd, metadata)
🎯 3 (Moderate) | ⏱️ ~25 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (3)
packages/openrouter-local/src/server/execute.ts (2)
54-71: ⚡ Quick winUse async file reading instead of blocking
readFileSync.The
executefunction is async, butreadFileSyncblocks the event loop. Usefs.promises.readFilefor non-blocking I/O.♻️ Proposed fix
if (config.instructionsFilePath) { try { - const fs = await import("node:fs"); - const instructions = fs.readFileSync(config.instructionsFilePath, "utf-8"); + const fs = await import("node:fs/promises"); + const instructions = await fs.readFile(config.instructionsFilePath, "utf-8"); messages.push({ role: "system", content: instructions, }); } catch (err) { return { exitCode: 1, signal: null, timedOut: false, - errorMessage: `Failed to read instructions file: ${config.instructionsFilePath}`, + errorMessage: `Failed to read instructions file: ${config.instructionsFilePath}: ${err instanceof Error ? err.message : String(err)}`, errorCode: "INSTRUCTIONS_READ_ERROR", }; } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/openrouter-local/src/server/execute.ts` around lines 54 - 71, The code in the async execute function currently uses blocking fs.readFileSync when reading config.instructionsFilePath; change this to non-blocking async reads by importing the promises API and awaiting the read: replace the dynamic import and readFileSync with either const fs = await import("node:fs/promises") and await fs.readFile(config.instructionsFilePath, "utf-8") or const fs = await import("node:fs") and await fs.promises.readFile(...), then push the result into messages as before; keep the existing try/catch around the await and return the same error object on failure (reference: execute function, config.instructionsFilePath, messages).
132-147: 💤 Low valueAuth errors should not be classified as transient.
Authentication failures (invalid API key) won't resolve on retry. Marking them as
transient_upstreamwith noretryNotBeforeis acceptable, but semantically auth errors are permanent until the key is fixed. Consider separating auth handling from rate limit handling.💡 Suggested separation
if (error.isAuthError() || error.isRateLimit()) { - // Transient errors that might succeed on retry + // Rate limit is transient; auth errors are permanent until config is fixed return { exitCode: 1, signal: null, timedOut: false, errorMessage: error.isAuthError() ? `OpenRouter authentication failed: ${error.message}. Check your API key.` : `OpenRouter rate limit exceeded: ${error.message}. Try again later.`, errorCode: error.isAuthError() ? "AUTH_ERROR" : "RATE_LIMIT", - errorFamily: "transient_upstream", + errorFamily: error.isRateLimit() ? "transient_upstream" : undefined, retryNotBefore: error.isRateLimit() ? new Date(Date.now() + 60000).toISOString() : undefined, }; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/openrouter-local/src/server/execute.ts` around lines 132 - 147, The code treats OpenRouterAuth errors as transient; update the OpenRouterApiError handling in the execute flow so auth failures (error.isAuthError()) are returned as a permanent upstream error (e.g., errorFamily "permanent_upstream" or similar, errorCode "AUTH_ERROR") with no retryNotBefore, while keeping rate-limit errors (error.isRateLimit()) as transient_upstream with a retryNotBefore timestamp; modify the conditional branching around OpenRouterApiError to separate the two paths (one for error.isAuthError() and one for error.isRateLimit()) and return the appropriate errorFamily, errorCode and retryNotBefore values accordingly.packages/openrouter-local/src/server/execute.test.ts (1)
4-24: 🏗️ Heavy liftTest file only covers metadata exports, not execution logic.
Despite being named
execute.test.ts, this file only tests static exports (type,label,models). Consider adding tests for:
execute()function with mocked API responses- Error handling paths (auth errors, rate limits, timeouts)
estimateCost()calculationstestEnvironment()connectivity checksThe free model test (Line 21) is also fragile as it assumes model IDs contain "deepseek" or "gemini" strings.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/openrouter-local/src/server/execute.test.ts` around lines 4 - 24, The test file execute.test.ts currently only asserts exported metadata and misses execution logic; add unit tests that exercise the execute(), estimateCost(), and testEnvironment() functions: write tests that mock HTTP/API responses (success responses, auth failures, rate-limit 429, and timeouts) and assert execute() returns expected outputs and throws/handles errors correctly, verify estimateCost() calculations with various input tokens and model parameters, and add connectivity tests for testEnvironment() using mocked network responses; also replace the fragile free-models check (which inspects models IDs for "deepseek" or "gemini") with a stable assertion—either check for a documented "isFree" property on models or compare against a maintained list of expected free model IDs—to avoid brittle string-matching.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/openrouter-local/README.md`:
- Around line 69-72: The README and code disagree on the free Gemini model ID;
update the pricing table in execute.ts so the zero-cost entry uses the same
model ID as the docs (`google/gemini-3.1-flash-lite`) (or alternatively change
the README to `google/gemini-2.0-flash-exp`) so both match; in practice locate
the pricing map/array in execute.ts where `google/gemini-2.0-flash-exp` is set
to zero and either rename that key to `google/gemini-3.1-flash-lite` or add an
equivalent zero-cost entry for `google/gemini-3.1-flash-lite`, and remove or
adjust the `google/gemini-2.0-flash-exp` entry to avoid ambiguity.
In `@packages/openrouter-local/src/index.ts`:
- Around line 4-19: The models array in packages/openrouter-local/src/index.ts
contains model ids that don't match the pricing keys used by estimateCost in
execute.ts (e.g., google/gemini-3.1-flash-lite vs google/gemini-2.0-flash-exp),
causing fallback to default pricing; fix by either updating the model id strings
in the exported models array to exactly match the pricing keys used by
estimateCost (ensure labels are updated to remain accurate) or add the missing
model ids to the estimateCost pricing table in execute.ts with the correct
pricing entries so every id in models has a corresponding pricing key; check
symbols: models (exported array) and estimateCost (pricing map) to reconcile ids
and labels.
- Around line 36-38: agentConfigurationDoc currently marks baseUrl as required
while execute.ts enforces it; make baseUrl optional and default it to
'https://openrouter.ai/api/v1' to match the README: update agentConfigurationDoc
to remove/flip the "required" flag for baseUrl, and in the execute.ts validation
logic that checks/reads baseUrl (the block that currently errors when baseUrl is
missing) set baseUrl = baseUrl ?? 'https://openrouter.ai/api/v1' and remove the
hard failure so missing baseUrl uses the default.
In `@packages/openrouter-local/src/server/execute.ts`:
- Around line 184-209: The function estimateCost declares return type number |
null but always returns a number; update the signature to return number (remove
| null) to match behavior, and ensure any callers or type imports referencing
estimateCost are updated accordingly; locate the function estimateCost and the
modelPricing fallback in this file to change the declared return type and keep
the existing pricing/default-fallback logic as-is.
- Around line 98-109: The code assumes choice.message and response.usage are
defined when computing output, finishReason and costEstimate; add defensive
null/undefined checks before accessing choice.message.content and response.usage
(e.g., verify choice && choice.message && typeof choice.message.content ===
"string" and response && response.usage) and provide safe fallbacks or early
returns/errors; then call estimateCost only when usage.prompt_tokens and
usage.completion_tokens exist, otherwise set costEstimate to null or 0. Update
the block that references choice.message, choice.finish_reason, response.usage,
config.model, and estimateCost to perform these guards and avoid uncaught
exceptions.
- Around line 222-232: The fetch call in this try block creates an
AbortController and a setTimeout but never clears the timer and uses a hardcoded
URL; change it to store the timeout id (e.g., const timeoutId = setTimeout(...))
and call clearTimeout(timeoutId) immediately after the awaited fetch (or in a
finally block) to avoid leaking timers, and replace the hardcoded
"https://openrouter.ai/api/v1/models" with a URL built from config.baseUrl
(e.g., `${config.baseUrl}/api/v1/models`) so custom endpoints are used; keep the
existing controller.signal for aborting and ensure the timeout/clear logic
surrounds the fetch in the try/finally that includes the AbortController and
timeoutId.
In `@packages/openrouter-local/src/server/openrouter-client.ts`:
- Around line 99-105: When calling response.json() in openrouter-client.ts (both
the non-2xx branch that throws OpenRouterApiError and the success path that
parses successful bodies), wrap the JSON parse in a try/catch so a
malformed/non-JSON body doesn't swallow the original HTTP status; if JSON
parsing fails, fall back to reading response.text() (or a generic message) and
throw/return using the original response.status and a sensible code (e.g.,
"unknown"), ensuring you construct the OpenRouterApiError with the preserved
response.status and include any available body text as the message; apply this
change around the existing response.json() calls and the OpenRouterApiError
constructor usage to retain upstream status codes like 401/429/502.
In `@packages/openrouter-local/src/server/test.ts`:
- Around line 108-110: The return uses an undefined identifier `type`; replace
it with a defined source for the adapter type (for example use an existing
`adapter` object's property like `adapter.type` or add/propagate a `adapterType`
parameter to the enclosing function) so `adapterType: ...` references a valid
identifier; update the surrounding function signature (e.g., add `adapterType:
string` or accept `adapter` and use `adapter.type`) and any call sites
accordingly to ensure TypeScript compiles.
In `@packages/openrouter-local/src/ui-parser.ts`:
- Around line 23-35: Parsed JSON objects that lack
choices?.[0]?.message?.content are currently ignored and silently dropped;
update the try block handling around parsed (the code that checks
parsed.choices?.[0]?.message?.content and pushes to entries) to add an else
branch that records the raw/unexpected payload so it surfaces in the UI
transcript: if parsed.error or parsed.message exists push an entry (e.g., type
"assistant_error" or "assistant_message") with content set to that error/message
and include metadata { model: parsed.model, usage: parsed.usage, raw:
JSON.stringify(parsed) } (or fall back to JSON.stringify(parsed) when no error
field), so unrecognized API responses are preserved instead of being discarded.
---
Nitpick comments:
In `@packages/openrouter-local/src/server/execute.test.ts`:
- Around line 4-24: The test file execute.test.ts currently only asserts
exported metadata and misses execution logic; add unit tests that exercise the
execute(), estimateCost(), and testEnvironment() functions: write tests that
mock HTTP/API responses (success responses, auth failures, rate-limit 429, and
timeouts) and assert execute() returns expected outputs and throws/handles
errors correctly, verify estimateCost() calculations with various input tokens
and model parameters, and add connectivity tests for testEnvironment() using
mocked network responses; also replace the fragile free-models check (which
inspects models IDs for "deepseek" or "gemini") with a stable assertion—either
check for a documented "isFree" property on models or compare against a
maintained list of expected free model IDs—to avoid brittle string-matching.
In `@packages/openrouter-local/src/server/execute.ts`:
- Around line 54-71: The code in the async execute function currently uses
blocking fs.readFileSync when reading config.instructionsFilePath; change this
to non-blocking async reads by importing the promises API and awaiting the read:
replace the dynamic import and readFileSync with either const fs = await
import("node:fs/promises") and await fs.readFile(config.instructionsFilePath,
"utf-8") or const fs = await import("node:fs") and await
fs.promises.readFile(...), then push the result into messages as before; keep
the existing try/catch around the await and return the same error object on
failure (reference: execute function, config.instructionsFilePath, messages).
- Around line 132-147: The code treats OpenRouterAuth errors as transient;
update the OpenRouterApiError handling in the execute flow so auth failures
(error.isAuthError()) are returned as a permanent upstream error (e.g.,
errorFamily "permanent_upstream" or similar, errorCode "AUTH_ERROR") with no
retryNotBefore, while keeping rate-limit errors (error.isRateLimit()) as
transient_upstream with a retryNotBefore timestamp; modify the conditional
branching around OpenRouterApiError to separate the two paths (one for
error.isAuthError() and one for error.isRateLimit()) and return the appropriate
errorFamily, errorCode and retryNotBefore values accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: aa5a3459-8b68-4168-898c-5c84651e1028
📒 Files selected for processing (13)
.changeset/openrouter-adapter-addition.mdREADME.mdpackages/openrouter-local/README.mdpackages/openrouter-local/package.jsonpackages/openrouter-local/src/index.tspackages/openrouter-local/src/server/execute.test.tspackages/openrouter-local/src/server/execute.tspackages/openrouter-local/src/server/index.tspackages/openrouter-local/src/server/openrouter-client.tspackages/openrouter-local/src/server/test.tspackages/openrouter-local/src/ui-parser.tspackages/openrouter-local/tsconfig.jsonpackages/openrouter-local/vitest.config.ts
| export const models: { id: string; label: string }[] = [ | ||
| // Free models | ||
| { id: "deepseek/deepseek-chat", label: "DeepSeek Chat (free)" }, | ||
| { id: "google/gemini-3.1-flash-lite", label: "Gemini 3.1 Flash Lite (free)" }, | ||
| { id: "meta-llama/llama-3.3-70b-instruct", label: "Llama 3.3 70B (free)" }, | ||
| { id: "qwen/qwen-2.5-72b-instruct", label: "Qwen 2.5 72B (free)" }, | ||
|
|
||
| // Cheap models | ||
| { id: "openai/gpt-4o-mini", label: "GPT-4o Mini ($0.15/M)" }, | ||
| { id: "anthropic/claude-3-haiku", label: "Claude 3 Haiku ($0.25/M)" }, | ||
|
|
||
| // Premium models | ||
| { id: "anthropic/claude-sonnet-4", label: "Claude Sonnet 4 ($3/M)" }, | ||
| { id: "openai/gpt-4o", label: "GPT-4o ($2.5/M)" }, | ||
| { id: "openai/o3", label: "OpenAI o3 ($10/M)" }, | ||
| ]; |
There was a problem hiding this comment.
Model list should align with estimateCost pricing table.
The models listed here (e.g., google/gemini-3.1-flash-lite) don't match the pricing keys in execute.ts (google/gemini-2.0-flash-exp). Users selecting these models won't get accurate cost estimates—they'll fall back to default pricing.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/openrouter-local/src/index.ts` around lines 4 - 19, The models array
in packages/openrouter-local/src/index.ts contains model ids that don't match
the pricing keys used by estimateCost in execute.ts (e.g.,
google/gemini-3.1-flash-lite vs google/gemini-2.0-flash-exp), causing fallback
to default pricing; fix by either updating the model id strings in the exported
models array to exactly match the pricing keys used by estimateCost (ensure
labels are updated to remain accurate) or add the missing model ids to the
estimateCost pricing table in execute.ts with the correct pricing entries so
every id in models has a corresponding pricing key; check symbols: models
(exported array) and estimateCost (pricing map) to reconcile ids and labels.
| Core fields: | ||
| - baseUrl (string, required): OpenRouter API base URL (default: https://openrouter.ai/api/v1) | ||
| - apiKey (string, required): Your OpenRouter API key (get at openrouter.ai/keys) |
There was a problem hiding this comment.
Documentation states baseUrl is required, but README shows it as optional with a default.
The agentConfigurationDoc says baseUrl is required, and execute.ts validates it as required. However, README line 59 lists it as optional with a default value. Either:
- Make
baseUrltruly optional with a default inexecute.ts, or - Update README to mark it as required
🔧 Option: Make baseUrl optional in execute.ts
// In execute.ts
- if (!config.baseUrl || !config.apiKey || !config.model) {
+ if (!config.apiKey || !config.model) {
return {
...
};
}
const client = new OpenRouterClient({
- baseUrl: config.baseUrl,
+ baseUrl: config.baseUrl || "https://openrouter.ai/api/v1",🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/openrouter-local/src/index.ts` around lines 36 - 38,
agentConfigurationDoc currently marks baseUrl as required while execute.ts
enforces it; make baseUrl optional and default it to
'https://openrouter.ai/api/v1' to match the README: update agentConfigurationDoc
to remove/flip the "required" flag for baseUrl, and in the execute.ts validation
logic that checks/reads baseUrl (the block that currently errors when baseUrl is
missing) set baseUrl = baseUrl ?? 'https://openrouter.ai/api/v1' and remove the
hard failure so missing baseUrl uses the default.
| try { | ||
| const controller = new AbortController(); | ||
| setTimeout(() => controller.abort(), 5000); | ||
|
|
||
| const response = await fetch("https://openrouter.ai/api/v1/models", { | ||
| method: "GET", | ||
| headers: { | ||
| "Authorization": `Bearer ${config.apiKey || "test"}`, | ||
| }, | ||
| signal: controller.signal, | ||
| }); |
There was a problem hiding this comment.
Timeout timer is never cleared; also URL should use config.baseUrl.
- The
setTimeouttimer leaks if fetch completes before 5s - The test uses a hardcoded URL instead of
config.baseUrl, so it won't validate custom endpoints
🔧 Proposed fix
try {
const controller = new AbortController();
- setTimeout(() => controller.abort(), 5000);
+ const timeoutId = setTimeout(() => controller.abort(), 5000);
- const response = await fetch("https://openrouter.ai/api/v1/models", {
+ const baseUrl = config.baseUrl || "https://openrouter.ai/api/v1";
+ const response = await fetch(`${baseUrl}/models`, {
method: "GET",
headers: {
"Authorization": `Bearer ${config.apiKey || "test"}`,
},
signal: controller.signal,
});
+ clearTimeout(timeoutId);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| try { | |
| const controller = new AbortController(); | |
| setTimeout(() => controller.abort(), 5000); | |
| const response = await fetch("https://openrouter.ai/api/v1/models", { | |
| method: "GET", | |
| headers: { | |
| "Authorization": `Bearer ${config.apiKey || "test"}`, | |
| }, | |
| signal: controller.signal, | |
| }); | |
| try { | |
| const controller = new AbortController(); | |
| const timeoutId = setTimeout(() => controller.abort(), 5000); | |
| const baseUrl = config.baseUrl || "https://openrouter.ai/api/v1"; | |
| const response = await fetch(`${baseUrl}/models`, { | |
| method: "GET", | |
| headers: { | |
| "Authorization": `Bearer ${config.apiKey || "test"}`, | |
| }, | |
| signal: controller.signal, | |
| }); | |
| clearTimeout(timeoutId); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/openrouter-local/src/server/execute.ts` around lines 222 - 232, The
fetch call in this try block creates an AbortController and a setTimeout but
never clears the timer and uses a hardcoded URL; change it to store the timeout
id (e.g., const timeoutId = setTimeout(...)) and call clearTimeout(timeoutId)
immediately after the awaited fetch (or in a finally block) to avoid leaking
timers, and replace the hardcoded "https://openrouter.ai/api/v1/models" with a
URL built from config.baseUrl (e.g., `${config.baseUrl}/api/v1/models`) so
custom endpoints are used; keep the existing controller.signal for aborting and
ensure the timeout/clear logic surrounds the fetch in the try/finally that
includes the AbortController and timeoutId.
| if (!response.ok) { | ||
| const errorData = (await response.json()) as OpenRouterError; | ||
| throw new OpenRouterApiError( | ||
| errorData.error.message, | ||
| response.status, | ||
| errorData.error.code | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
OpenRouter /chat/completions API: Are non-2xx responses always JSON with error.messageanderror.code, or can gateways/proxies return non-JSON bodies?
💡 Result:
No. OpenRouter’s intended error shape for successful JSON parsing is JSON, but OpenRouter (and especially intermediaries/gateways) can return bodies that are not valid JSON (or otherwise don’t match the expected error.message / error.code structure). For example, there are reports of “Unexpected end of JSON input” when calling /api/v1/chat/completions, indicating non-JSON or truncated JSON responses can occur in practice.[1] What OpenRouter documents (for Chat Completions) is nuanced: - OpenRouter documents structured JSON error handling for Chat Completions, with differences depending on timing/streaming: “No tokens sent” returns a standalone ErrorResponse, “Some tokens sent” embeds error info within choices, and “Streaming: Errors sent as SSE events with top-level error field.”[2] - However, this documents the normal/expected contract, not the behavior when something goes wrong at the HTTP/gateway layer or when the upstream response is truncated/invalid. So for your question: non-2xx responses are not guaranteed to always be JSON with error.message and error.code. Code defensively: 1) Check response.ok / status first. 2) Attempt await response.text() and only JSON.parse if it looks like JSON. 3) Fall back to HTTP status + raw text when parsing fails. Grounding examples: - OpenRouter intends an error { code, message } structure for structured errors.[2] - But a real-world client issue shows OpenRouter sometimes returns invalid/truncated JSON from /chat/completions (triggering JSON parse failures).[1]
Citations:
- 1: OpenRouter Error: "Invalid JSON response body at https://openrouter.ai/api/v1/chat/completions Reason: Unexpected end of JSON input" cline/cline#60
- 2: https://openrouter.ai/docs/api/reference/errors-and-debugging
🏁 Script executed:
cat -n packages/openrouter-local/src/server/openrouter-client.ts | sed -n '85,135p'Repository: superbiche/paperclip-adapters
Length of output: 1965
Preserve upstream HTTP status even when error body is non-JSON.
The non-2xx handler (line 100) calls await response.json() without protection. If the response body is non-JSON, truncated, or malformed—which occurs in practice with OpenRouter—the JSON parse exception is caught at line 110 and remapped to { status: 500, code: "unknown" } at lines 122–125. This loses the real upstream status (e.g., 401, 429, 502), breaking auth and rate-limit handling.
Web search confirms that OpenRouter and intermediaries can return non-JSON or truncated bodies, despite documented JSON error contracts.
Suggested fix
if (!response.ok) {
- const errorData = (await response.json()) as OpenRouterError;
- throw new OpenRouterApiError(
- errorData.error.message,
- response.status,
- errorData.error.code
- );
+ const raw = await response.text();
+ let message = `OpenRouter request failed with status ${response.status}`;
+ let code: string | undefined;
+
+ if (raw) {
+ try {
+ const errorData = JSON.parse(raw) as OpenRouterError;
+ message = errorData.error?.message ?? message;
+ code = errorData.error?.code;
+ } catch {
+ message = raw.slice(0, 500);
+ }
+ }
+
+ throw new OpenRouterApiError(message, response.status, code);
}Also applies to line 108 (success path) where response.json() can similarly fail and drop status fidelity in the same catch handler.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/openrouter-local/src/server/openrouter-client.ts` around lines 99 -
105, When calling response.json() in openrouter-client.ts (both the non-2xx
branch that throws OpenRouterApiError and the success path that parses
successful bodies), wrap the JSON parse in a try/catch so a malformed/non-JSON
body doesn't swallow the original HTTP status; if JSON parsing fails, fall back
to reading response.text() (or a generic message) and throw/return using the
original response.status and a sensible code (e.g., "unknown"), ensuring you
construct the OpenRouterApiError with the preserved response.status and include
any available body text as the message; apply this change around the existing
response.json() calls and the OpenRouterApiError constructor usage to retain
upstream status codes like 401/429/502.
- test.ts: Fix undefined 'type' import, add clearTimeout - execute.ts: Add null checks for choice.message and response.usage - execute.ts: Fix estimateCost return type (number, not number | null) - execute.ts: Update pricing table to match docs (gemini-3.1-flash-lite) - README.md: Clarify baseUrl as optional with default, fix model IDs - ui-parser.ts: Handle non-JSON error responses, don't drop unrecognized lines All CodeRabbit suggestions addressed. Ready for review!
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/openrouter-local/src/server/test.ts (1)
63-74: ⚡ Quick winSkip connectivity probing when required config is already invalid.
If
apiKeyormodelis missing, the result is already a fail. Running the probe anyway adds avoidable network calls and noisy secondary checks.Suggested minimal guard
- // Test API connectivity (optional, non-blocking) - const controller = new AbortController(); - const timeoutId = setTimeout(() => controller.abort(), 5000); - try { + // Test API connectivity only when required fields are present + if (apiKey && model) { + const controller = new AbortController(); + const timeoutId = setTimeout(() => controller.abort(), 5000); + try { const response = await fetch(`${baseUrl}/models`, { @@ - } catch (error) { - checks.push({ - level: "warn", - message: `Could not connect to OpenRouter API: ${error instanceof Error ? error.message : "Unknown error"}`, - hint: "Check your network connection and firewall settings", - code: "api_connection_failed", - }); - } finally { - clearTimeout(timeoutId); + } catch (error) { + checks.push({ + level: "warn", + message: `Could not connect to OpenRouter API: ${error instanceof Error ? error.message : "Unknown error"}`, + hint: "Check your network connection and firewall settings", + code: "api_connection_failed", + }); + } finally { + clearTimeout(timeoutId); + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/openrouter-local/src/server/test.ts` around lines 63 - 74, Add an early guard in the connectivity probe in test.ts to skip the network fetch when required config is already invalid: check the relevant config variables (apiKey and model) before creating the AbortController or calling fetch, and return/skip the probe if either is missing; update the logic around the controller/timeoutId and the fetch to only run when apiKey && model are present so you avoid unnecessary network calls and noisy errors.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/openrouter-local/README.md`:
- Around line 55-66: Remove the duplicated "Optional Fields" heading and table:
keep a single "Optional Fields" section and consolidate the rows so `baseUrl`
appears only once and `timeoutSec` is included (i.e., merge the two tables into
one containing `baseUrl` and `timeoutSec`), removing the redundant heading and
duplicate `baseUrl` entry.
In `@packages/openrouter-local/src/server/test.ts`:
- Around line 64-77: The timeout cleanup (clearTimeout(timeoutId)) is only
executed after a successful fetch and can leak if fetch throws; move
clearTimeout(timeoutId) into a finally block that always runs after the
try/await so the timeout is cleared even on errors — wrap the fetch call and
response handling in try { ... } finally { clearTimeout(timeoutId); } using the
existing AbortController/timeoutId variables (and apply the same change to the
other occurrence around the second fetch at the later block that currently uses
controller/timeoutId).
---
Nitpick comments:
In `@packages/openrouter-local/src/server/test.ts`:
- Around line 63-74: Add an early guard in the connectivity probe in test.ts to
skip the network fetch when required config is already invalid: check the
relevant config variables (apiKey and model) before creating the AbortController
or calling fetch, and return/skip the probe if either is missing; update the
logic around the controller/timeoutId and the fetch to only run when apiKey &&
model are present so you avoid unnecessary network calls and noisy errors.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a7577f4e-8fe5-472d-8c28-67fbe4a52504
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (4)
packages/openrouter-local/README.mdpackages/openrouter-local/src/server/execute.tspackages/openrouter-local/src/server/test.tspackages/openrouter-local/src/ui-parser.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/openrouter-local/src/server/execute.ts
| ### Optional Fields | ||
|
|
||
| | Field | Type | Default | Description | | ||
| |-------|------|---------|-------------| | ||
| | `baseUrl` | string | `https://openrouter.ai/api/v1` | OpenRouter API base URL | | ||
|
|
||
| ### Optional Fields | ||
|
|
||
| | Field | Type | Default | Description | | ||
| |-------|------|---------|-------------| | ||
| | `baseUrl` | string | `https://openrouter.ai/api/v1` | OpenRouter API base URL | | ||
| | `timeoutSec` | number | `120` | Request timeout in seconds | |
There was a problem hiding this comment.
Remove the duplicated Optional Fields section.
Line 61 repeats the same heading from Line 55, and baseUrl is duplicated (Line 59 and Line 65). Keep a single table to avoid doc drift and duplicate TOC anchors.
Suggested minimal diff
-### Optional Fields
-
-| Field | Type | Default | Description |
-|-------|------|---------|-------------|
-| `baseUrl` | string | `https://openrouter.ai/api/v1` | OpenRouter API base URL |
-
### Optional Fields
| Field | Type | Default | Description |
|-------|------|---------|-------------|
| `baseUrl` | string | `https://openrouter.ai/api/v1` | OpenRouter API base URL |📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ### Optional Fields | |
| | Field | Type | Default | Description | | |
| |-------|------|---------|-------------| | |
| | `baseUrl` | string | `https://openrouter.ai/api/v1` | OpenRouter API base URL | | |
| ### Optional Fields | |
| | Field | Type | Default | Description | | |
| |-------|------|---------|-------------| | |
| | `baseUrl` | string | `https://openrouter.ai/api/v1` | OpenRouter API base URL | | |
| | `timeoutSec` | number | `120` | Request timeout in seconds | | |
| ### Optional Fields | |
| | Field | Type | Default | Description | | |
| |-------|------|---------|-------------| | |
| | `baseUrl` | string | `https://openrouter.ai/api/v1` | OpenRouter API base URL | | |
| | `timeoutSec` | number | `120` | Request timeout in seconds | |
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 61-61: Multiple headings with the same content
(MD024, no-duplicate-heading)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/openrouter-local/README.md` around lines 55 - 66, Remove the
duplicated "Optional Fields" heading and table: keep a single "Optional Fields"
section and consolidate the rows so `baseUrl` appears only once and `timeoutSec`
is included (i.e., merge the two tables into one containing `baseUrl` and
`timeoutSec`), removing the redundant heading and duplicate `baseUrl` entry.
| try { | ||
| const controller = new AbortController(); | ||
| const timeoutId = setTimeout(() => controller.abort(), 5000); | ||
|
|
||
| const response = await fetch(`${baseUrl}/models`, { | ||
| method: "GET", | ||
| headers: { | ||
| "Authorization": `Bearer ${apiKey || "test"}`, | ||
| }, | ||
| signal: controller.signal, | ||
| }); | ||
|
|
||
| clearTimeout(timeoutId); | ||
|
|
There was a problem hiding this comment.
Always clear the timeout in a finally block.
clearTimeout(timeoutId) currently runs only after a successful fetch (Line 76). If fetch throws, the timer remains scheduled. Move cleanup to finally.
Suggested fix
- // Test API connectivity (optional, non-blocking)
- try {
- const controller = new AbortController();
- const timeoutId = setTimeout(() => controller.abort(), 5000);
-
- const response = await fetch(`${baseUrl}/models`, {
+ // Test API connectivity (optional, non-blocking)
+ const controller = new AbortController();
+ const timeoutId = setTimeout(() => controller.abort(), 5000);
+ try {
+ const response = await fetch(`${baseUrl}/models`, {
method: "GET",
headers: {
"Authorization": `Bearer ${apiKey || "test"}`,
},
signal: controller.signal,
});
-
- clearTimeout(timeoutId);
-
+
if (response.ok) {
checks.push({
level: "info",
@@
} catch (error) {
checks.push({
level: "warn",
message: `Could not connect to OpenRouter API: ${error instanceof Error ? error.message : "Unknown error"}`,
hint: "Check your network connection and firewall settings",
code: "api_connection_failed",
});
+ } finally {
+ clearTimeout(timeoutId);
}Also applies to: 98-105
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/openrouter-local/src/server/test.ts` around lines 64 - 77, The
timeout cleanup (clearTimeout(timeoutId)) is only executed after a successful
fetch and can leak if fetch throws; move clearTimeout(timeoutId) into a finally
block that always runs after the try/await so the timeout is cleared even on
errors — wrap the fetch call and response handling in try { ... } finally {
clearTimeout(timeoutId); } using the existing AbortController/timeoutId
variables (and apply the same change to the other occurrence around the second
fetch at the later block that currently uses controller/timeoutId).
|
Thanks for the contribution. The supply-chain posture stands out before the substance — zero new runtime deps, plain Decision: request changes. The architectural premise needs a call (B1 below), and there are 10 blockers + 9 significant findings + 5 minors to address before merge. The big call firstB1 — One-shot chat-completion wrapper vs. agent adapter. The PR body's framing ("engineers running DeepSeek for free, CEO/CTO on Claude Sonnet 4, $1,200-1,700/year savings") implies engineers can actually code with these models through the adapter. As built, they cannot, because the model has no way to touch the workspace. Two legitimate paths, your call:
The findings below apply on either path. BlockersB2 + B3 — B4 — Root README claim broken on merge. The diff at B5 — Cost estimation wrong on three axes (
Fix: drop the pricing table, send B6 — Error-response schema mismatch. B7 — B8 — Dead/duplicate B9 — B10 — SignificantS2 — Sync file I/O in async path. S3 — Prompt-assembly path skipped. S4 — Lagging dep pin. S5 — Dead S6 — S7 — Timeouts misreported as generic API errors. Client throws S8 — S9 — Auth errors miscategorized as Minor5 minor items
Repo conventions
House norm — AI disclosureHeads up on a house norm: when a PR body, README sections, or substantial code are AI-drafted, please disclose. Several artifacts in this PR are tells of paste-without-reread — the README "Cost Impact" section, the duplicate Not asking you to stop using AI — asking you to read it before sending and to disclose when you do. Same norm I apply to my own AI-drafted comments (signed in the footer below). This is not a moral judgment; fabricated details treated as your considered position cost reviewers cycles they do not get back. On the review workflowFor future PRs to this repo, the cadence that respects everyone's time: CodeRabbit posts findings → you read and resolve them (or reply with rationale where you disagree) → then you ping me. The Discord ping I got — "CodeRabbit review clean, just waiting on @superbiche's eyes" — went out while your two CR reviews on this PR each had "Actionable comments posted" counts in bold at the top (9 and 2). Several of those overlap with what I flagged above. Resolving them before the ping would have cut my review time roughly in half and put iteration cost on the side where it belongs. Not blocking this PR over it; flagging the convention for next time. Happy to clarify any specific finding or walk through how the sibling adapters handle a particular concern. No rush from my side on the rework. @superbiche · maintainer · drafted with Claude Opus 4.7, reviewed before posting; the voice and decisions are mine. |
2 participants
Thinking Path
openrouter_aiadapter with token tracking and cost estimationWhat Changed
Added
@superbiche/openrouter-paperclip-adapterpackages/openrouter-local/Features
Verification
Tested
Risks
@paperclipai/adapter-utils(already required)Model Used
Checklist
Cost Impact
Before (Claude Max subscription):
After (OpenRouter with mixed models):
Built by @fatherkam for the Paperclip community. Happy to iterate based on feedback! 💋
Summary by CodeRabbit
New Features
Documentation
Tests
Packaging