fix: fall back to direct provider API when OpenRouter key is missing#2
Conversation
When a user has GEMINI_API_KEY, OPENAI_API_KEY, or ANTHROPIC_API_KEY set but not OPENROUTER_API_KEY, aliases like --model gemini now automatically route through the direct provider API instead of failing with "OPENROUTER_API_KEY required". Adds applyDirectApiFallback() which strips the openrouter/ prefix from alias-resolved models when the direct provider key is available. OpenRouter is still preferred when both keys are present. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When the direct API fallback triggers (no OpenRouter key), validate that the model actually exists on the provider before proceeding. If validation fails: - Interactive mode: prompts user to pick from available models, saves choice to config.json so they're never prompted again - Headless mode: fails with available models list and fix command Adds model-validator.js with validateDirectModel() using the existing model-fetcher.js infrastructure. Network errors skip validation gracefully (don't block the user). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds direct-API fallback handling for model alias resolution, a pre-start validation path for fallback targets (interactive and headless), config loading/hashing improvements, a new model-validator module, Deepseek key support, CLI flag --validate-model, and expanded tests for these flows. Changes
Sequence Diagram(s)sequenceDiagram
participant Sidecar as Sidecar.js
participant Config as Config Resolver
participant Validator as Model Validator
participant Provider as Provider API
participant User as User
Sidecar->>Config: resolveModel(rawAlias)
Config->>Config: applyDirectApiFallback(alias)
Config-->>Sidecar: resolvedModel
Sidecar->>Config: detectFallback(alias, resolvedModel)
Config-->>Sidecar: fallback? (true/false)
alt Fallback Detected
Sidecar->>Validator: validateDirectModel(resolvedModel, alias, {headless?/ui})
Validator->>Provider: fetch available models (uses provider API key)
Provider-->>Validator: model list
Validator->>Validator: filterRelevantModels(models, alias)
alt Match Found
Validator-->>Sidecar: validatedResolvedModel
else No Match & Interactive
Validator->>User: present options & prompt
User-->>Validator: selection
Validator->>Config: persist alias -> model mapping
Validator-->>Sidecar: selectedValidatedModel
else No Match & Headless
Validator-->>Sidecar: throw detailed error (includes suggestions)
end
else No Fallback
Sidecar->>Sidecar: continue startup with resolvedModel
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
jrenaldi79
left a comment
jrenaldi79
left a comment
There was a problem hiding this comment.
Thanks for tackling this! The core routing fix is solid and matches what I arrived at independently.
A few notes:
1. Duplicate provider key map
DIRECT_API_KEYS in config.js duplicates PROVIDER_ENV_MAP from api-key-store.js (which already has the same mapping). Consider importing and reusing it:
const { PROVIDER_ENV_MAP } = require('./api-key-store');You'd just need to add deepseek to PROVIDER_ENV_MAP (it's currently missing there too).
2. Model validation is a nice addition, but a couple of concerns:
- It introduces a network call (
fetchModelsFromProvider) in the critical startup path. If the provider API is slow or down, this adds latency to every fallback invocation. Consider making it opt-in or caching aggressively. - The interactive
readlineprompt inmodel-validator.jswon't work when sidecar is invoked as an MCP server (stdin is the transport). Worth guarding againstclient === 'cowork'or similar.
3. Magic number: model.slice(11) — prefer model.slice('openrouter/'.length) for readability.
4. Test cleanup: The beforeEach in config.test.js now clears all API key env vars globally, which is good for the new tests but could mask issues in existing tests that previously relied on whatever env state was present. The afterEach restore via process.env = originalEnv handles it, but worth double-checking no test ordering issues crept in.
Overall the approach is right — happy to iterate on these points.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@bin/sidecar.js`:
- Around line 124-139: Preserve the raw user-provided model alias before calling
resolveModel so detectFallback can see default aliases: store the pre-resolution
value (e.g., rawAlias = args.model) before calling resolveModel(args.model),
then call detectFallback(rawAlias, args.model) (instead of
detectFallback(originalAlias, args.model)); if rawAlias is undefined, derive a
sensible alias from the resolved model (or config.default) so detectFallback
treats default aliases the same as explicit ones and validateDirectModel
(validateDirectModel) runs when appropriate.
In `@src/utils/model-validator.js`:
- Around line 116-119: loadConfig() currently returning null for
unreadable/malformed config is being treated like a missing file and causes
saveConfig(...) to overwrite the broken config with only aliases; change the
logic around the use of loadConfig in the block that sets config.aliases so that
if loadConfig() returns null but the config file actually exists the function
fails instead of proceeding: call loadConfig() into a variable, detect the
"malformed" case (config === null) and if fs.existsSync/config path check shows
the file exists, throw an error or surface a clear failure; otherwise (file
truly missing) initialize config = {} and continue to set config.aliases[alias]
= newModel and call saveConfig(config). Ensure you reference loadConfig,
saveConfig, config.aliases, alias and newModel when implementing the check.
- Around line 59-68: The code currently only checks options.headless before
deciding to throw vs calling promptModelSelection; update the guard so
non-interactive TTYs are treated as headless too by checking Node's TTY state
(e.g. process.stdin.isTTY and process.stdout.isTTY). In the block that currently
reads "if (options.headless) { ... throw new Error(...)}" (referencing
options.headless, relevant, modelId, provider, alias), change the condition to
also consider !process.stdin.isTTY || !process.stdout.isTTY (or compute an
isInteractive flag) and keep the same error/available-models message; only call
promptModelSelection(relevant, alias, provider, modelId) when the session is
actually interactive.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3d16f294-13aa-45ce-a290-355786e049d3
📒 Files selected for processing (5)
bin/sidecar.jssrc/utils/config.jssrc/utils/model-validator.jstests/config.test.jstests/model-validator.test.js
|
@ellisjr Thanks for the solid PR! Between our review and CodeRabbit's inline findings, here's a consolidated list of action items before we can merge: Must fix
Nice to have
Let us know if you have questions on any of these — happy to help iterate. |
- Make --validate-model opt-in instead of always-on (feedback jrenaldi79#2) - Add TTY guard in both bin/sidecar.js and model-validator.js (feedback jrenaldi79#3/7) - Detect default alias when --model is omitted (feedback jrenaldi79#6) - Replace duplicate DIRECT_API_KEYS with PROVIDER_ENV_MAP import (feedback jrenaldi79#4) - Add deepseek to PROVIDER_ENV_MAP (feedback #1) - Use 'openrouter/'.length instead of magic number 11 (feedback jrenaldi79#4) - Add malformed config safety in promptModelSelection (feedback jrenaldi79#8) - Add tests for TTY guard and malformed config behavior Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
All feedback addressed in commit 9a84730:
3 new tests added for TTY guard and malformed config behavior. All 120 config + model-validator tests pass. |
jrenaldi79
left a comment
jrenaldi79
left a comment
There was a problem hiding this comment.
Thanks for the quick turnaround on the feedback, @ellisjr! The third commit addresses several items well (TTY guard, config overwrite protection, PROVIDER_ENV_MAP reuse, magic number fix). A few remaining items before we can merge:
1. Silent fallback with zero logging (critical)
applyDirectApiFallback() silently reroutes the model with no logging or user feedback. When the direct provider rejects the model (different naming conventions between OpenRouter and direct APIs, or an expired key), users get opaque downstream errors with no idea their model was rerouted.
This is especially important for MCP callers (Cowork, Claude Desktop) who get the silent fallback with no validation path.
Fix: Add logger.warn() when the fallback activates:
if (envVar && process.env[envVar]) {
logger.warn({ msg: 'Using direct provider API (OPENROUTER_API_KEY not set)', original: model, resolved: direct });
return direct;
}2. --validate-model flag not registered in cli.js
The diff doesn't show any changes to src/cli.js to register --validate-model in parseArgs. If the arg parser uses strict known-flags, this flag gets silently dropped and the entire validation feature is dead code. Please verify it's wired up and add a cli.test.js test for it.
3. Default path needs a notice when validation is skipped
Fallback is always-on but validation is opt-in. When a user hits the fallback path without --validate-model, they should at minimum see a stderr notice:
Notice: Using direct google API (OPENROUTER_API_KEY not set). Use --validate-model to verify model availability.
Without this, the default experience is: alias silently rerouted → provider rejects unknown model → confusing error.
4. saveConfig() can crash after interactive selection
In promptModelSelection, saveConfig(config) can throw (EACCES, ENOSPC, etc.) and there's no try/catch. The user goes through the interactive picker, selects a model, and gets an unhandled crash. Wrap it so the selected model is still returned for the current session:
try {
saveConfig(config);
process.stderr.write(` Saved: ${alias} → ${newModel}\n`);
} catch (err) {
process.stderr.write(` Warning: Could not save selection (${err.message}). Using for this session only.\n`);
}5. deepseek missing from VALIDATION_ENDPOINTS
deepseek was added to PROVIDER_ENV_MAP but not to VALIDATION_ENDPOINTS in api-key-store.js. This means validateApiKey('deepseek', key) will always report invalid if that code path is hit.
Everything else looks good — the core routing logic is clean, the headless/interactive split is well-designed, and the malformed config detection is solid. Happy to merge once these are addressed.
- Add logger.warn + stderr notice when direct API fallback activates (#1) - Register --validate-model as boolean flag in cli.js, add to usage text (jrenaldi79#2) - Add stderr notice pointing users to --validate-model on fallback (jrenaldi79#3) - Wrap saveConfig in try/catch in promptModelSelection (jrenaldi79#4) - Add deepseek to VALIDATION_ENDPOINTS in api-key-store.js (jrenaldi79#5) - Add CLI tests for --validate-model flag - Suppress stderr in config fallback tests Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Addressed all 5 items from the latest review in commits 8a7419e and 7da6918:
|
…ests - Add model param to mcp-server sidecar_start test calls (resolveModel now requires a model or configured default) - Update api-key-store provider count assertion from 4 to 5 (deepseek) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/utils/api-key-store.js`:
- Around line 14-15: The default objects returned by readApiKeys() and
readApiKeyHints() currently only seed four providers so the newly added provider
key "deepseek" is missing; update both readApiKeys and readApiKeyHints to
include an explicit "deepseek" entry (initialize to false or the same default
value other providers use) so callers iterating Object.keys(result) will see
DeepSeek and callers get false rather than undefined. Locate readApiKeys and
readApiKeyHints in src/utils/api-key-store.js and add the "deepseek" property to
their seeded/default return objects to match the updated provider map.
In `@src/utils/config.js`:
- Around line 87-100: applyDirectApiFallback currently only checks process.env
for provider API keys, so saved keys in Sidecar's persisted store
(~/.config/sidecar/.env) are ignored; update applyDirectApiFallback to consult
the persisted key store in addition to process.env when computing envVar (use
PROVIDER_ENV_MAP[...]) — e.g., add a helper (loadPersistedKeys/getPersistedKey)
and treat a found persisted key as equivalent to process.env[envVar] when
deciding to return direct; keep the existing logger.warn/process.stderr notice
behavior when falling back and reference the same symbols
(applyDirectApiFallback, PROVIDER_ENV_MAP, logger.warn) so resolution works when
users saved keys via Sidecar.
In `@src/utils/model-validator.js`:
- Around line 54-64: The code may persist or suggest bare model IDs (e.g.,
'gemini-2.5-flash') because found checks both resolvedModel and modelId but m.id
may lack a provider prefix; update places that return or suggest model
identifiers (the branch returning resolvedModel and the suggestion using
relevant[0]?.id in the headless/error message) to always emit a
provider-qualified ID: if the candidate id does not include a '/', prefix it
with `${provider}/` before returning or showing it. Apply the same normalization
logic wherever model IDs are persisted or suggested (including the similar block
around lines 113-136) and use filterRelevantModels/relevant as the source but
ensure provider-qualification before writing to config or showing the fix hint.
In `@tests/api-key-store.test.js`:
- Around line 57-58: The test currently added a fifth provider but didn't
include DEEPSEEK_API_KEY in the setup and assertions; update the test that
references PROVIDER_ENV_MAP to ensure process.env.DEAPSEEK_API_KEY
(DEEPSEEK_API_KEY) is explicitly cleared/undefined in the setup (e.g.,
beforeEach) and change the baseline "no keys" expectations to include the
'deepseek' provider so the suite asserts deepseek: false by default; locate
references to PROVIDER_ENV_MAP and add 'deepseek' into the keys/assertions and
ensure process.env.DEPPSEEK_API_KEY is unset before running the assertions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ad8517d7-0469-457b-8ff5-473c7fd40691
📒 Files selected for processing (8)
src/cli.jssrc/utils/api-key-store.jssrc/utils/config.jssrc/utils/model-validator.jstests/api-key-store.test.jstests/cli.test.jstests/config.test.jstests/mcp-server.test.js
| anthropic: 'ANTHROPIC_API_KEY', | ||
| deepseek: 'DEEPSEEK_API_KEY' |
There was a problem hiding this comment.
Initialize deepseek in the default return objects.
readApiKeys() and readApiKeyHints() still seed only four providers. With this map change, both helpers return undefined instead of false for DeepSeek until a key is set, and callers iterating Object.keys(result) will never see the new provider.
Suggested fix
function readApiKeys() {
- const result = { openrouter: false, google: false, openai: false, anthropic: false };
+ const result = Object.fromEntries(
+ Object.keys(PROVIDER_ENV_MAP).map(provider => [provider, false])
+ );
const entries = loadEnvEntries();
for (const [provider, envVar] of Object.entries(PROVIDER_ENV_MAP)) {
if (resolveKeyValue(entries, envVar)) { result[provider] = true; }
}
return result;
}
@@
function readApiKeyHints() {
- const result = { openrouter: false, google: false, openai: false, anthropic: false };
+ const result = Object.fromEntries(
+ Object.keys(PROVIDER_ENV_MAP).map(provider => [provider, false])
+ );
const entries = loadEnvEntries();
for (const [provider, envVar] of Object.entries(PROVIDER_ENV_MAP)) {
const key = resolveKeyValue(entries, envVar);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/utils/api-key-store.js` around lines 14 - 15, The default objects
returned by readApiKeys() and readApiKeyHints() currently only seed four
providers so the newly added provider key "deepseek" is missing; update both
readApiKeys and readApiKeyHints to include an explicit "deepseek" entry
(initialize to false or the same default value other providers use) so callers
iterating Object.keys(result) will see DeepSeek and callers get false rather
than undefined. Locate readApiKeys and readApiKeyHints in
src/utils/api-key-store.js and add the "deepseek" property to their
seeded/default return objects to match the updated provider map.
| /** Strip openrouter/ prefix when direct provider API key is available but OPENROUTER_API_KEY is not */ | ||
| function applyDirectApiFallback(model) { | ||
| if (!model.startsWith('openrouter/') || process.env.OPENROUTER_API_KEY) { | ||
| return model; | ||
| } | ||
| const direct = model.slice('openrouter/'.length); | ||
| const envVar = PROVIDER_ENV_MAP[direct.split('/')[0]]; | ||
| if (envVar && process.env[envVar]) { | ||
| logger.warn({ msg: 'Using direct provider API (OPENROUTER_API_KEY not set)', original: model, resolved: direct }); | ||
| process.stderr.write( | ||
| `Notice: Using direct ${direct.split('/')[0]} API (OPENROUTER_API_KEY not set). ` + | ||
| 'Use --validate-model to verify model availability.\n' | ||
| ); | ||
| return direct; |
There was a problem hiding this comment.
Use the persisted key store when deciding direct fallback.
applyDirectApiFallback() only checks process.env. Users who saved GEMINI_API_KEY/OPENAI_API_KEY/ANTHROPIC_API_KEY via Sidecar have those keys in ~/.config/sidecar/.env, not necessarily exported in the shell, so built-in aliases still resolve to openrouter/... and fail when OPENROUTER_API_KEY is absent.
Suggested fix
-const { PROVIDER_ENV_MAP } = require('./api-key-store');
+const { PROVIDER_ENV_MAP, readApiKeyValues } = require('./api-key-store');
@@
function applyDirectApiFallback(model) {
if (!model.startsWith('openrouter/') || process.env.OPENROUTER_API_KEY) {
return model;
}
const direct = model.slice('openrouter/'.length);
- const envVar = PROVIDER_ENV_MAP[direct.split('/')[0]];
- if (envVar && process.env[envVar]) {
+ const provider = direct.split('/')[0];
+ const envVar = PROVIDER_ENV_MAP[provider];
+ const keyValues = readApiKeyValues();
+ if (envVar && keyValues[provider]) {
logger.warn({ msg: 'Using direct provider API (OPENROUTER_API_KEY not set)', original: model, resolved: direct });
process.stderr.write(
- `Notice: Using direct ${direct.split('/')[0]} API (OPENROUTER_API_KEY not set). ` +
+ `Notice: Using direct ${provider} API (OPENROUTER_API_KEY not set). ` +
'Use --validate-model to verify model availability.\n'
);
return direct;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/utils/config.js` around lines 87 - 100, applyDirectApiFallback currently
only checks process.env for provider API keys, so saved keys in Sidecar's
persisted store (~/.config/sidecar/.env) are ignored; update
applyDirectApiFallback to consult the persisted key store in addition to
process.env when computing envVar (use PROVIDER_ENV_MAP[...]) — e.g., add a
helper (loadPersistedKeys/getPersistedKey) and treat a found persisted key as
equivalent to process.env[envVar] when deciding to return direct; keep the
existing logger.warn/process.stderr notice behavior when falling back and
reference the same symbols (applyDirectApiFallback, PROVIDER_ENV_MAP,
logger.warn) so resolution works when users saved keys via Sidecar.
| const found = models.some(m => m.id === resolvedModel || m.id === modelId); | ||
| if (found) { return resolvedModel; } | ||
|
|
||
| const relevant = filterRelevantModels(models, alias); | ||
|
|
||
| if (options.headless || !process.stdin.isTTY) { | ||
| const list = relevant.slice(0, 10).map(m => ` ${m.id}`).join('\n'); | ||
| throw new Error( | ||
| `Model '${modelId}' not found on ${provider} API.\n` + | ||
| `Available models:\n${list}\n` + | ||
| `Fix with: sidecar setup --add-alias ${alias}=${relevant[0]?.id || 'provider/model'}` |
There was a problem hiding this comment.
Persist a provider-qualified model ID.
found already accepts both resolvedModel and bare modelId, so m.id is not guaranteed to include the provider prefix. Saving or suggesting raw selected.id can write values like gemini-2.5-flash into config.json, which later fails provider/model validation on the next run.
Suggested fix
+function normalizeModelId(provider, id) {
+ return id.includes('/') ? id : `${provider}/${id}`;
+}
+
async function validateDirectModel(resolvedModel, alias, options = {}) {
@@
if (options.headless || !process.stdin.isTTY) {
- const list = relevant.slice(0, 10).map(m => ` ${m.id}`).join('\n');
+ const list = relevant
+ .slice(0, 10)
+ .map(m => ` ${normalizeModelId(provider, m.id)}`)
+ .join('\n');
throw new Error(
`Model '${modelId}' not found on ${provider} API.\n` +
`Available models:\n${list}\n` +
- `Fix with: sidecar setup --add-alias ${alias}=${relevant[0]?.id || 'provider/model'}`
+ `Fix with: sidecar setup --add-alias ${alias}=${
+ relevant[0] ? normalizeModelId(provider, relevant[0].id) : `${provider}/model`
+ }`
);
}
@@
- const newModel = selected.id;
+ const newModel = normalizeModelId(provider, selected.id);Also applies to: 113-136
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/utils/model-validator.js` around lines 54 - 64, The code may persist or
suggest bare model IDs (e.g., 'gemini-2.5-flash') because found checks both
resolvedModel and modelId but m.id may lack a provider prefix; update places
that return or suggest model identifiers (the branch returning resolvedModel and
the suggestion using relevant[0]?.id in the headless/error message) to always
emit a provider-qualified ID: if the candidate id does not include a '/', prefix
it with `${provider}/` before returning or showing it. Apply the same
normalization logic wherever model IDs are persisted or suggested (including the
similar block around lines 113-136) and use filterRelevantModels/relevant as the
source but ensure provider-qualification before writing to config or showing the
fix hint.
| it('should have exactly 5 providers', () => { | ||
| expect(Object.keys(PROVIDER_ENV_MAP)).toHaveLength(5); |
There was a problem hiding this comment.
Cover the DeepSeek slot in setup and baseline assertions.
After increasing this to five providers, the suite still leaves DEEPSEEK_API_KEY untouched and the “no keys” expectations still omit deepseek. That makes the new provider environment-dependent and would not catch a missing deepseek: false default.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/api-key-store.test.js` around lines 57 - 58, The test currently added a
fifth provider but didn't include DEEPSEEK_API_KEY in the setup and assertions;
update the test that references PROVIDER_ENV_MAP to ensure
process.env.DEAPSEEK_API_KEY (DEEPSEEK_API_KEY) is explicitly cleared/undefined
in the setup (e.g., beforeEach) and change the baseline "no keys" expectations
to include the 'deepseek' provider so the suite asserts deepseek: false by
default; locate references to PROVIDER_ENV_MAP and add 'deepseek' into the
keys/assertions and ensure process.env.DEPPSEEK_API_KEY is unset before running
the assertions.
jrenaldi79
left a comment
jrenaldi79
left a comment
There was a problem hiding this comment.
All feedback addressed. Approving — we'll handle the remaining follow-up items (CLAUDE.md update, file size limits, persisted key detection, model ID normalization) in a separate PR.
Address CodeRabbit review items and code quality issues from PR #2: - Check persisted keys (not just process.env) in applyDirectApiFallback - Normalize model IDs before saving to config (prepend provider prefix) - Seed deepseek in readApiKeys/readApiKeyHints default objects - Add DeepSeek test coverage to api-key-store tests - Update CLAUDE.md for new model-validator and model-fetcher modules - Extract handleStart helpers to stay under 50-line function limit - Split config.test.js (825→4 files) to stay under 300-line file limit Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix: post-merge follow-ups for direct API fallback (#2) Address CodeRabbit review items and code quality issues from PR #2: - Check persisted keys (not just process.env) in applyDirectApiFallback - Normalize model IDs before saving to config (prepend provider prefix) - Seed deepseek in readApiKeys/readApiKeyHints default objects - Add DeepSeek test coverage to api-key-store tests - Update CLAUDE.md for new model-validator and model-fetcher modules - Extract handleStart helpers to stay under 50-line function limit - Split config.test.js (825→4 files) to stay under 300-line file limit Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: address PR review feedback (CodeRabbit + Claude) Bugs fixed: - Normalize model IDs in headless error path (model-validator.js) - Wrap autoRepairAlias saveConfig in try/catch for best-effort persistence File size violations resolved: - bin/sidecar.js 434→283 lines (extracted start-helpers.js, cli-handlers.js) - config.js 350→299 lines (extracted alias-resolver.js) - api-key-store.test.js 659→3 files, all under 300 lines - model-validator.test.js 344→2 files, all under 300 lines Minor fixes: - addAlias rejects 'null' name and trims whitespace - Tightened config-null-alias.test.js assertions to check exact defaults - Added un-mocked integration test for persisted key fallback - Updated CLAUDE.md with normalizeModelId + 5 new test files Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: add try/catch for malformed metadata.json in handleAbort Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
2 participants
Summary
GEMINI_API_KEY,OPENAI_API_KEY, orANTHROPIC_API_KEYset but notOPENROUTER_API_KEY, aliases like--model gemininow automatically route through the direct provider API instead of failing with "OPENROUTER_API_KEY required"config.jsonso they're never prompted againProblem
All built-in aliases (
gemini,gpt,opus, etc.) hardcoded toopenrouter/...model paths. Users with direct provider API keys but no OpenRouter key got:Changes
src/utils/config.js— AddedapplyDirectApiFallback()to stripopenrouter/prefix when direct provider key is available, anddetectFallback()to detect when this occurredsrc/utils/model-validator.js— New module: validates fallback models exist on the provider viamodel-fetcher.js. Interactive prompt on failure, actionable error in headless modebin/sidecar.js— Hooks validation intohandleStart()after model resolutionconfig.test.jsandmodel-validator.test.jsTest plan
npx jest tests/config.test.js tests/model-validator.test.js)GEMINI_API_KEY, unsetOPENROUTER_API_KEY, runsidecar start --model gemini --prompt "hello" --no-ui~/.config/sidecar/config.jsonand subsequent runs skip the prompt🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes / Reliability
Tests