diff --git a/docs/superpowers/plans/2026-05-16-pr18-review-fixes.md b/docs/superpowers/plans/2026-05-16-pr18-review-fixes.md new file mode 100644 index 0000000..a510177 --- /dev/null +++ b/docs/superpowers/plans/2026-05-16-pr18-review-fixes.md @@ -0,0 +1,950 @@ +# PR #18 Review Issues Implementation Plan + +> **For agentic workers:** REQUIRED: Use superpowers:subagent-driven-development (if subagents available) or superpowers:executing-plans to implement this plan. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Address all 20 review issues from PR #18 (excluding CHANGELOG entry) + +**Architecture:** Systematic fixes across 6 source files and 2 test files, organized by severity and file grouping. Medium-priority issues first, then low-priority. + +**Tech Stack:** TypeScript ESM, Node.js native test runner, native fetch + +--- + +## File Structure + +**Modified files:** +- `src/plugin.ts` — 6 issues (fallback operator, metadata validation, magic constants, as const, tool defaults, array ordering) +- `src/models.ts` — 4 issues (cache key, sensitive logs, DRY violation, candidate explosion) +- `src/models-dev.ts` — 2 issues (metadata tracking, trailing newline) +- `src/omniroute-combos.ts` — 2 issues (log injection, DRY splitModelId) +- `src/types.ts` — 2 issues (provider fields shape, variants type) +- `src/logger.ts` — 1 issue (synchronous I/O) +- `test/models.test.mjs` — 3 test gaps (temperature/reasoning, variant+alias, subscription fallback) +- `test/plugin.test.mjs` — 1 issue (hardcoded ports) + +--- + +## Chunk 1: Critical Fixes (Medium Priority) + +### Task 1: Fix API Key Fallback Operator + +**Files:** +- Modify: `src/plugin.ts:45` + +**Issue:** `||` treats empty string `""` as falsy and falls back to env var unexpectedly. + +- [ ] **Step 1: Update fallback operator** + +```typescript +// Change from: +const apiKey = auth?.key || process.env.OMNIROUTE_API_KEY; +// To: +const apiKey = auth?.key ?? process.env.OMNIROUTE_API_KEY; +``` + +- [ ] **Step 2: Verify no other `||` fallback patterns exist** + +Run: `grep -n "|| process.env" src/*.ts` +Expected: No matches (or only intentional ones) + +- [ ] **Step 3: Commit** + +```bash +git add src/plugin.ts +git commit -m "fix: use nullish coalescing for API key fallback" +``` + +--- + +### Task 2: Add Runtime Validation for modelMetadata + +**Files:** +- Modify: `src/plugin.ts:351-379` (mergeModelMetadata function) +- Modify: `src/types.ts` (if needed for validator types) + +**Issue:** `metadata as OmniRouteModelMetadata` casts without validation. + +- [ ] **Step 1: Create metadata validation helper** + +Add to `src/plugin.ts` after `isRecord` function: + +```typescript +function isValidModelMetadata(value: unknown): value is OmniRouteModelMetadata { + if (!isRecord(value)) return false; + + // Only validate boolean fields if present + const booleanFields = [ + 'supportsStreaming', 'supportsVision', 'supportsTools', + 'supportsTemperature', 'supportsReasoning', 'supportsAttachment' + ]; + + for (const field of booleanFields) { + if (field in value && typeof value[field] !== 'boolean') { + return false; + } + } + + // Validate numeric fields + if ('contextWindow' in value && typeof value.contextWindow !== 'number') return false; + if ('maxTokens' in value && typeof value.maxTokens !== 'number') return false; + + return true; +} +``` + +- [ ] **Step 2: Update mergeModelMetadata to validate** + +```typescript +// In mergeModelMetadata, replace: +merged[id] = { + ...(generated[id] ?? {}), + ...(metadata as OmniRouteModelMetadata), +}; +// With: +if (isValidModelMetadata(metadata)) { + merged[id] = { + ...(generated[id] ?? {}), + ...metadata, + }; +} else { + warn(`Invalid metadata for model "${id}", skipping`); +} +``` + +- [ ] **Step 3: Build and test** + +Run: `npm run build` +Expected: No errors + +Run: `npm test` +Expected: All tests pass + +- [ ] **Step 4: Commit** + +```bash +git add src/plugin.ts +git commit -m "fix: add runtime validation for modelMetadata merge" +``` + +--- + +### Task 3: Fix Cache Key to Include modelsDev Config + +**Files:** +- Modify: `src/models.ts:36-39` (getCacheKey function) + +**Issue:** Different `modelsDev` configs share same cache key. + +- [ ] **Step 1: Update getCacheKey to hash config** + +```typescript +function getCacheKey(config: OmniRouteConfig, apiKey: string): string { + const baseUrl = config.baseUrl || OMNIROUTE_ENDPOINTS.BASE_URL; + + // Include modelsDev config in cache key + const modelsDevHash = config.modelsDev + ? JSON.stringify({ + enabled: config.modelsDev.enabled, + url: config.modelsDev.url, + providerAliases: config.modelsDev.providerAliases, + }) + : ''; + + return `${baseUrl}:${apiKey}:${modelsDevHash}`; +} +``` + +- [ ] **Step 2: Add test for cache key differentiation** + +In `test/models.test.mjs`, add: + +```javascript +test('fetchModels uses different cache for different modelsDev configs', async () => { + let calls = 0; + global.fetch = async (input) => { + const url = input instanceof Request ? input.url : input.toString(); + if (url.includes('/v1/models')) { + calls++; + return new Response( + JSON.stringify({ object: 'list', data: [{ id: `model-${calls}`, name: `Model ${calls}` }] }), + { status: 200, headers: { 'Content-Type': 'application/json' } } + ); + } + return new Response(JSON.stringify({ data: [] }), { status: 200 }); + }; + + const config1 = { ...CONFIG, modelsDev: { enabled: true, providerAliases: { oai: 'openai' } } }; + const config2 = { ...CONFIG, modelsDev: { enabled: true, providerAliases: { oai: 'anthropic' } } }; + + await fetchModels(config1, CONFIG.apiKey, false); + await fetchModels(config2, CONFIG.apiKey, false); + + assert.equal(calls, 2, 'Should fetch twice for different modelsDev configs'); +}); +``` + +- [ ] **Step 3: Run tests** + +Run: `npm test` +Expected: All tests pass + +- [ ] **Step 4: Commit** + +```bash +git add src/models.ts test/models.test.mjs +git commit -m "fix: include modelsDev config in cache key" +``` + +--- + +### Task 4: Fix DRY Violation - Extract splitModelId + +**Files:** +- Modify: `src/omniroute-combos.ts:227-248` — Export the function +- Modify: `src/models.ts:340-359` — Remove duplicate, import and use + +**Issue:** `splitModelId` and `splitOmniRouteModelForLookup` are identical. + +- [ ] **Step 1: Export splitModelId from omniroute-combos.ts** + +Change: +```typescript +function splitModelId(...) { ... } +``` +To: +```typescript +export function splitModelId(...) { ... } +``` + +- [ ] **Step 2: Remove duplicate from models.ts and import** + +In `src/models.ts`: +1. Add to imports from `omniroute-combos.ts`: +```typescript +import { splitModelId } from './omniroute-combos.js'; +``` + +2. Remove `splitOmniRouteModelForLookup` function entirely. + +3. Replace usage: +```typescript +const { providerKey, modelKey } = splitOmniRouteModelForLookup(model.id); +``` +With: +```typescript +const { providerKey, modelKey } = splitModelId(model.id); +``` + +- [ ] **Step 3: Build and test** + +Run: `npm run build` +Expected: No errors + +Run: `npm test` +Expected: All tests pass + +- [ ] **Step 4: Commit** + +```bash +git add src/models.ts src/omniroute-combos.ts +git commit -m "refactor: extract splitModelId to eliminate DRY violation" +``` + +--- + +### Task 5: Fix supportsTools Default Inconsistency + +**Files:** +- Modify: `src/plugin.ts:432` (toProviderModel) +- Modify: `src/models-dev.ts:331` (calculateLowestCommonCapabilities) + +**Issue:** `toProviderModel` defaults tools to true, combo logic defaults to false. + +- [ ] **Step 1: Document and align the defaults** + +In `src/plugin.ts`, add comment: +```typescript +// Default to true: if API doesn't explicitly say no tools, assume capability exists +// This aligns with OpenAI-compatible behavior where most models support tools +const supportsTools = model.supportsTools !== false; +``` + +In `src/models-dev.ts`, update combo logic: +```typescript +// For combos: only advertise tools if ALL underlying models explicitly support them +// This is intentionally stricter than single-model defaults because a combo +// with one tool-less model cannot reliably use tools across all backends +const supportsTools = model.tool_call === true; +``` + +- [ ] **Step 2: Commit** + +```bash +git add src/plugin.ts src/models-dev.ts +git commit -m "docs: document supportsTools default rationale" +``` + +--- + +### Task 6: Fix Array Metadata Ordering + +**Files:** +- Modify: `src/plugin.ts:351-379` (mergeModelMetadata) + +**Issue:** Generated blocks prepended — verify if OpenCode uses first or last match wins. + +- [ ] **Step 1: Research OpenCode matching logic** + +Check `@opencode-ai/plugin` source or documentation for `modelMetadata` array processing. + +- [ ] **Step 2: If last-match-wins, reverse order** + +```typescript +// If OpenCode uses last-match-wins, user config should come last +return [...userConfig, ...generatedBlocks]; +``` + +- [ ] **Step 3: Commit** + +```bash +git add src/plugin.ts +git commit -m "fix: correct modelMetadata array ordering for OpenCode matching" +``` + +--- + +### Task 7: Verify Provider Model Fields Match OpenCode Shape + +**Files:** +- Modify: `src/types.ts:111-155` + +**Issue:** New fields may not match `@opencode-ai/plugin` expectations. + +- [ ] **Step 1: Check OpenCode plugin types** + +Run: `npm ls @opencode-ai/plugin` or check node_modules for exact interface. + +- [ ] **Step 2: Rename fields if needed** + +If `tool_call` should be `toolCall`: +```typescript +// In OmniRouteProviderModel: +toolCall?: boolean; // instead of tool_call +``` + +Update all references in `src/plugin.ts` accordingly. + +- [ ] **Step 3: Commit** + +```bash +git add src/types.ts src/plugin.ts +git commit -m "fix: align provider model fields with OpenCode plugin types" +``` + +--- + +### Task 8: Tighten Variants Type + +**Files:** +- Modify: `src/types.ts:154` + +**Issue:** `Record` is too permissive. + +- [ ] **Step 1: Define strict variant types** + +```typescript +export interface OmniRouteModelVariant { + reasoningEffort?: 'low' | 'medium' | 'high'; + // Future variant types can be added here +} + +// In OmniRouteProviderModel: +variants: Record; +``` + +- [ ] **Step 2: Update toProviderModel** + +```typescript +variants: supportsReasoning + ? { + low: { reasoningEffort: 'low' }, + medium: { reasoningEffort: 'medium' }, + high: { reasoningEffort: 'high' }, + } + : {}, +``` + +- [ ] **Step 3: Build and test** + +Run: `npm run build` +Expected: No type errors + +- [ ] **Step 4: Commit** + +```bash +git add src/types.ts src/plugin.ts +git commit -m "types: tighten variants type to prevent invalid reasoning effort values" +``` + +--- + +## Chunk 2: Low Priority Fixes + +### Task 9: Fix Sensitive Data in Log + +**Files:** +- Modify: `src/models.ts:102` + +**Issue:** `JSON.stringify(rawData)` could leak sensitive data. + +- [ ] **Step 1: Replace with shape-only logging** + +```typescript +// Change from: +warn(`Invalid models response structure: ${JSON.stringify(rawData)}`); +// To: +const dataType = rawData && typeof rawData === 'object' + ? (Array.isArray(rawData.data) ? 'array' : typeof rawData.data) + : typeof rawData; +warn(`Invalid models response structure: expected { data: Array }, got { data: ${dataType} }`); +``` + +- [ ] **Step 2: Commit** + +```bash +git add src/models.ts +git commit -m "fix: avoid logging sensitive response data in models fetch error" +``` + +--- + +### Task 10: Fix Log Injection via Model IDs + +**Files:** +- Modify: `src/omniroute-combos.ts:55` and other log lines + +**Issue:** Model IDs from external API interpolated without sanitization. + +- [ ] **Step 1: Add sanitization helper** + +```typescript +function sanitizeForLog(value: string): string { + return value.replace(/[\r\n]/g, ''); +} +``` + +- [ ] **Step 2: Apply to all interpolated model IDs** + +Search for `model.id` in log lines and wrap with sanitizeForLog: +```typescript +warn(`Could not resolve ${unresolvedModels.length} underlying models for "${sanitizeForLog(model.id)}": ${unresolvedModels.map(sanitizeForLog).join(', ')}`); +``` + +- [ ] **Step 3: Commit** + +```bash +git add src/omniroute-combos.ts +git commit -m "fix: sanitize model IDs in log messages to prevent injection" +``` + +--- + +### Task 11: Fix Synchronous File I/O in Logger + +**Files:** +- Modify: `src/logger.ts` + +**Issue:** `appendFileSync` blocks event loop. + +- [ ] **Step 1: Switch to async logging** + +```typescript +// Replace appendFileSync with fire-and-forget appendFile +import { appendFile } from 'fs/promises'; + +export function warn(message: string): void { + const logFile = getLogFile(); + if (!logFile) return; + + const line = formatLogLine('WARN', message); + // Fire-and-forget: don't await, don't crash on error + appendFile(logFile, line).catch(() => {}); +} + +export function debug(message: string): void { + if (process.env.OMNIROUTE_DEBUG !== '1') return; + + const logFile = getLogFile(); + if (!logFile) return; + + const line = formatLogLine('DEBUG', message); + appendFile(logFile, line).catch(() => {}); +} +``` + +- [ ] **Step 2: Commit** + +```bash +git add src/logger.ts +git commit -m "perf: use async file I/O for logger to prevent event loop blocking" +``` + +--- + +### Task 12: Optimize Candidate Explosion in Lookup + +**Files:** +- Modify: `src/models.ts:290-307` + +**Issue:** Up to 8 candidates per model key, ~19k lookups. + +- [ ] **Step 1: Add early-exit for aliases that resolve to same key** + +```typescript +function getModelLookupCandidates(modelKey: string): string[] { + const candidates = new Set(); + + const addCandidate = (key: string): void => { + const lower = key.toLowerCase(); + const normalized = normalizeModelKey(key); + const aliasResolved = resolveModelAlias(key); + + candidates.add(lower); + candidates.add(normalized); + + // Only add alias variants if they differ from original + if (aliasResolved !== key) { + candidates.add(aliasResolved.toLowerCase()); + candidates.add(normalizeModelKey(aliasResolved)); + } + }; + + addCandidate(modelKey); + + const { base, stripped } = stripVariantSuffix(modelKey); + if (stripped) { + addCandidate(base); + } + + return [...candidates]; +} +``` + +- [ ] **Step 2: Commit** + +```bash +git add src/models.ts +git commit -m "perf: avoid duplicate lookup candidates when alias resolves to same key" +``` + +--- + +### Task 13: Fix Magic Constant 4096 + +**Files:** +- Modify: `src/plugin.ts:486-487` +- Modify: `src/constants.ts` — Add constants + +**Issue:** `4096` used without named constant. + +- [ ] **Step 1: Add named constants** + +In `src/constants.ts`: +```typescript +export const DEFAULT_CONTEXT_LIMIT = 4096; +export const DEFAULT_OUTPUT_LIMIT = 4096; +``` + +- [ ] **Step 2: Update plugin.ts to use constants** + +```typescript +import { DEFAULT_CONTEXT_LIMIT, DEFAULT_OUTPUT_LIMIT } from './constants.js'; + +// In toProviderModel: +limit: { + context: model.contextWindow ?? DEFAULT_CONTEXT_LIMIT, + output: model.maxTokens ?? DEFAULT_OUTPUT_LIMIT, +}, +``` + +- [ ] **Step 3: Commit** + +```bash +git add src/constants.ts src/plugin.ts +git commit -m "refactor: extract magic constant 4096 to named defaults" +``` + +--- + +### Task 14: Remove Redundant `as const` Assertions + +**Files:** +- Modify: `src/plugin.ts:447-449` + +**Issue:** `as const` is redundant given the type definition. + +- [ ] **Step 1: Remove assertions** + +```typescript +// Change from: +modalities: { + input: supportsVision ? ['text', 'image'] as const : ['text'] as const, + output: ['text'] as const, +}, + +// To: +modalities: { + input: supportsVision ? ['text', 'image'] : ['text'], + output: ['text'], +}, +``` + +- [ ] **Step 2: Verify types still compile** + +Run: `npx tsc --noEmit src/plugin.ts` +Expected: No errors + +- [ ] **Step 3: Commit** + +```bash +git add src/plugin.ts +git commit -m "style: remove redundant as const assertions" +``` + +--- + +### Task 15: Add Test for Single Model Metadata Tracking + +**Files:** +- Modify: `test/models.test.mjs` + +**Issue:** Early return bypasses metadata tracking logic. + +- [ ] **Step 1: Add test verifying consistency** + +```javascript +test('calculateLowestCommonCapabilities produces identical output for single model and combo-with-self', () => { + const single = calculateLowestCommonCapabilities([ + { id: 'test-model', temperature: true, reasoning: true, attachment: true } + ]); + + const combo = calculateLowestCommonCapabilities([ + { id: 'test-model', temperature: true, reasoning: true, attachment: true }, + { id: 'test-model', temperature: true, reasoning: true, attachment: true } + ]); + + assert.deepEqual(single, combo); +}); +``` + +- [ ] **Step 2: Run tests** + +Run: `npm test` +Expected: All tests pass + +- [ ] **Step 3: Commit** + +```bash +git add test/models.test.mjs +git commit -m "test: verify single-model and combo-with-self produce identical capabilities" +``` + +--- + +### Task 16: Add Trailing Newline to models-dev.ts + +**Files:** +- Modify: `src/models-dev.ts:480` + +**Issue:** File ends without trailing newline. + +- [ ] **Step 1: Add newline** + +Simply ensure the file ends with a newline character. + +- [ ] **Step 2: Commit** + +```bash +git add src/models-dev.ts +git commit -m "style: add trailing newline to models-dev.ts" +``` + +--- + +## Chunk 3: Test Coverage Gaps + +### Task 17: Add Temperature/Reasoning Combo Tests + +**Files:** +- Modify: `test/models.test.mjs` + +**Issue:** Missing tests for temperature and reasoning logic. + +- [ ] **Step 1: Add temperature tests** + +```javascript +test('calculateLowestCommonCapabilities handles temperature with mixed defined/undefined', () => { + const capabilities = calculateLowestCommonCapabilities([ + { id: 'with-temp', temperature: true }, + { id: 'without-temp' }, + ]); + + assert.equal(capabilities.supportsTemperature, true); +}); + +test('calculateLowestCommonCapabilities respects explicit temperature false', () => { + const capabilities = calculateLowestCommonCapabilities([ + { id: 'with-temp', temperature: true }, + { id: 'no-temp', temperature: false }, + ]); + + assert.equal(capabilities.supportsTemperature, false); +}); + +test('calculateLowestCommonCapabilities handles all three capabilities together', () => { + const capabilities = calculateLowestCommonCapabilities([ + { id: 'full', temperature: true, reasoning: true, attachment: true }, + { id: 'limited', temperature: false, reasoning: false, attachment: false }, + ]); + + assert.equal(capabilities.supportsTemperature, false); + assert.equal(capabilities.supportsReasoning, false); + assert.equal(capabilities.supportsAttachment, false); +}); + +test('calculateLowestCommonCapabilities handles single model with undefined temperature', () => { + const capabilities = calculateLowestCommonCapabilities([ + { id: 'no-meta' }, + ]); + + assert.equal(capabilities.supportsTemperature, undefined); +}); +``` + +- [ ] **Step 2: Run tests** + +Run: `npm test` +Expected: All tests pass + +- [ ] **Step 3: Commit** + +```bash +git add test/models.test.mjs +git commit -m "test: add temperature and reasoning combo capability tests" +``` + +--- + +### Task 18: Add Variant+Alias Integration Tests + +**Files:** +- Modify: `test/models.test.mjs` + +**Issue:** No test for variant suffix stripping + alias resolution. + +- [ ] **Step 1: Add integration test** + +```javascript +test('variant suffix stripping with alias resolution works end-to-end', async () => { + global.fetch = async (input) => { + const url = input instanceof Request ? input.url : input.toString(); + if (url.includes('/v1/models')) { + return new Response( + JSON.stringify({ + object: 'list', + data: [ + { id: 'moonshotai/kimi-k2.6-thinking-high', name: 'Kimi K2.6 Thinking High' } + ] + }), + { status: 200, headers: { 'Content-Type': 'application/json' } } + ); + } + if (url.includes('models.dev')) { + return new Response( + JSON.stringify({ + moonshotai: { + models: { + 'kimi-k2-thinking': { + id: 'kimi-k2-thinking', + name: 'Kimi K2 Thinking', + temperature: true, + reasoning: true, + } + } + } + }), + { status: 200, headers: { 'Content-Type': 'application/json' } } + ); + } + return new Response(JSON.stringify({ data: [] }), { status: 200 }); + }; + + const models = await fetchModels(CONFIG, CONFIG.apiKey, true); + const model = models.find(m => m.id === 'moonshotai/kimi-k2.6-thinking-high'); + + assert.ok(model, 'Model should be found'); + assert.equal(model.supportsReasoning, true, 'Should resolve alias and enrich capabilities'); +}); +``` + +- [ ] **Step 2: Run tests** + +Run: `npm test` +Expected: All tests pass + +- [ ] **Step 3: Commit** + +```bash +git add test/models.test.mjs +git commit -m "test: add variant suffix and alias resolution integration test" +``` + +--- + +### Task 19: Add Subscription Fallback Tests + +**Files:** +- Modify: `test/models.test.mjs` + +**Issue:** No test for subscription provider fallback. + +- [ ] **Step 1: Add subscription fallback test** + +```javascript +test('subscription fallback enriches from public provider', async () => { + global.fetch = async (input) => { + const url = input instanceof Request ? input.url : input.toString(); + if (url.includes('/v1/models')) { + return new Response( + JSON.stringify({ + object: 'list', + data: [ + { id: 'zai-coding-plan/gpt-4o', name: 'GPT-4o via ZAI' } + ] + }), + { status: 200, headers: { 'Content-Type': 'application/json' } } + ); + } + if (url.includes('models.dev')) { + return new Response( + JSON.stringify({ + zai: { + models: { + 'gpt-4o': { + id: 'gpt-4o', + name: 'GPT-4o', + temperature: true, + tool_call: true, + } + } + } + }), + { status: 200, headers: { 'Content-Type': 'application/json' } } + ); + } + return new Response(JSON.stringify({ data: [] }), { status: 200 }); + }; + + const models = await fetchModels(CONFIG, CONFIG.apiKey, true); + const model = models.find(m => m.id === 'zai-coding-plan/gpt-4o'); + + assert.ok(model, 'Model should be found'); + assert.equal(model.supportsTemperature, true, 'Should fallback to zai provider'); + assert.equal(model.supportsTools, true, 'Should inherit tools from fallback'); +}); +``` + +- [ ] **Step 2: Run tests** + +Run: `npm test` +Expected: All tests pass + +- [ ] **Step 3: Commit** + +```bash +git add test/models.test.mjs +git commit -m "test: add subscription fallback provider test" +``` + +--- + +### Task 20: Fix Hardcoded Ports in Tests + +**Files:** +- Modify: `test/plugin.test.mjs` + +**Issue:** Hardcoded ports like `20129`, `20130`, `20131`. + +- [ ] **Step 1: Add test helper for dummy URLs** + +At top of test file: +```javascript +function getDummyBaseUrl(port = 20128) { + return `http://localhost:${port}/v1`; +} +``` + +- [ ] **Step 2: Replace hardcoded ports with helper** + +Search for `http://localhost:20` and replace with `getDummyBaseUrl()` calls. + +- [ ] **Step 3: Commit** + +```bash +git add test/plugin.test.mjs +git commit -m "test: replace hardcoded ports with helper function" +``` + +--- + +## Chunk 4: Final Verification + +### Task 21: Full Build and Test + +- [ ] **Step 1: Clean build** + +Run: `npm run clean && npm run build` +Expected: Success + +- [ ] **Step 2: Run all tests** + +Run: `npm test` +Expected: All tests pass (target: 40+ tests) + +- [ ] **Step 3: Type check** + +Run: `npx tsc --noEmit` +Expected: No errors + +- [ ] **Step 4: Lint check** + +Run: `npm run lint` (if available) +Expected: No errors + +- [ ] **Step 5: Final commit summary** + +```bash +git log --oneline -20 +``` + +--- + +## Summary + +**Total tasks:** 21 (20 fixes + 1 verification) +**Estimated commits:** 20 +**Estimated time:** 2-3 hours + +### Priority Order: +1. **Medium priority first** (Tasks 1-8): Core functionality and type safety +2. **Low priority fixes** (Tasks 9-16): Logging, performance, style +3. **Test coverage** (Tasks 17-20): Fill testing gaps +4. **Verification** (Task 21): Ensure everything works + +### Key Decisions: +- Use `??` instead of `||` for API key fallback +- Keep `supportsTools` defaults as-is but document rationale +- Export `splitModelId` from `omniroute-combos.ts` +- Include `modelsDev` config in cache key hash +- Tighten `variants` type to `OmniRouteModelVariant` +- Add comprehensive test coverage for temperature/reasoning combos diff --git a/src/models-dev.ts b/src/models-dev.ts index e88e13c..1f3c983 100644 --- a/src/models-dev.ts +++ b/src/models-dev.ts @@ -246,6 +246,18 @@ export function modelsDevToMetadata(model: ModelsDevModel): OmniRouteModelMetada metadata.maxTokens = model.limit.output; } + if (model.temperature !== undefined) { + metadata.supportsTemperature = model.temperature; + } + + if (model.reasoning !== undefined) { + metadata.supportsReasoning = model.reasoning; + } + + if (model.attachment !== undefined) { + metadata.supportsAttachment = model.attachment; + } + // Derive vision support from modalities if (model.modalities?.input?.includes('image')) { metadata.supportsVision = true; @@ -256,8 +268,6 @@ export function modelsDevToMetadata(model: ModelsDevModel): OmniRouteModelMetada metadata.supportsTools = true; } - - // Pricing if (model.cost?.input !== undefined || model.cost?.output !== undefined) { metadata.pricing = {}; @@ -292,6 +302,12 @@ export function calculateLowestCommonCapabilities( let minMaxTokens: number | undefined; let allSupportVision = true; let allSupportTools = true; + let allSupportTemperature = true; + let hasTemperatureMetadata = false; + let allSupportReasoning = true; + let hasReasoningMetadata = false; + let allSupportAttachment = true; + let hasAttachmentMetadata = false; let allSupportStreaming = true; for (const model of models) { @@ -314,6 +330,21 @@ export function calculateLowestCommonCapabilities( // Tools: all must support it const supportsTools = model.tool_call === true; allSupportTools = allSupportTools && supportsTools; + + if (model.temperature !== undefined) { + hasTemperatureMetadata = true; + allSupportTemperature = allSupportTemperature && model.temperature; + } + + if (model.reasoning !== undefined) { + hasReasoningMetadata = true; + allSupportReasoning = allSupportReasoning && model.reasoning; + } + + if (model.attachment !== undefined) { + hasAttachmentMetadata = true; + allSupportAttachment = allSupportAttachment && model.attachment; + } } const result: OmniRouteModelMetadata = {}; @@ -334,6 +365,24 @@ export function calculateLowestCommonCapabilities( result.supportsTools = true; } + if (hasTemperatureMetadata && allSupportTemperature) { + result.supportsTemperature = true; + } else if (hasTemperatureMetadata) { + result.supportsTemperature = false; + } + + if (hasReasoningMetadata && allSupportReasoning) { + result.supportsReasoning = true; + } else if (hasReasoningMetadata) { + result.supportsReasoning = false; + } + + if (hasAttachmentMetadata && allSupportAttachment) { + result.supportsAttachment = true; + } else if (hasAttachmentMetadata) { + result.supportsAttachment = false; + } + // Streaming is generally supported by all modern models if (allSupportStreaming) { result.supportsStreaming = true; @@ -343,6 +392,31 @@ export function calculateLowestCommonCapabilities( } +/** + * Subscription → public provider fallback map. + * When a subscription provider (e.g. zai-coding-plan) lacks a model, + * try its public counterpart (e.g. zai) before giving up. + */ +export const SUBSCRIPTION_FALLBACKS: Record = { + 'zai-coding-plan': 'zai', + 'kimi-for-coding': 'moonshotai', + 'github-models': 'google', +}; + +/** + * Known model ID mismatches between OmniRoute and models.dev. + * Maps OmniRoute model names to their models.dev equivalents. + */ +export const MODEL_ALIASES: Record = { + 'kimi-k2.6-thinking': 'kimi-k2-thinking', + 'kimi-k2.6-thinking-turbo': 'kimi-k2-thinking-turbo', +}; + +export function resolveModelAlias(modelKey: string): string { + const lower = modelKey.toLowerCase(); + return MODEL_ALIASES[lower] ?? MODEL_ALIASES[normalizeModelKey(lower)] ?? modelKey; +} + /** * Resolve provider alias using config and defaults */ @@ -372,8 +446,35 @@ export function resolveProviderAlias( openrouter: 'openrouter', perplexity: 'perplexity', cohere: 'cohere', + glmt: 'zai-coding-plan', + glm: 'zai-coding-plan', + 'kimi-coding': 'moonshotai', + kmc: 'moonshotai', + gh: 'google', + github: 'google', ...config?.modelsDev?.providerAliases, }; return aliases[lower] ?? lower; -} \ No newline at end of file +} + +/** + * Get the public fallback provider for a subscription provider. + * Returns null if no fallback exists. + */ +export function getSubscriptionFallback(provider: string): string | null { + return SUBSCRIPTION_FALLBACKS[provider.toLowerCase()] ?? null; +} + +/** + * Strip reasoning effort variant suffix from a model name. + * Returns the base model name and true if a suffix was stripped. + */ +export function stripVariantSuffix(modelKey: string): { base: string; stripped: boolean } { + const variantPattern = /-(low|medium|high|xhigh)$/i; + const match = modelKey.match(variantPattern); + if (match) { + return { base: modelKey.slice(0, match.index), stripped: true }; + } + return { base: modelKey, stripped: false }; +} diff --git a/src/models.ts b/src/models.ts index 4eed8f0..4c0759e 100644 --- a/src/models.ts +++ b/src/models.ts @@ -1,13 +1,20 @@ -import type { OmniRouteConfig, OmniRouteModel, OmniRouteModelMetadata, OmniRouteModelsResponse } from './types.js'; +import type { OmniRouteConfig, OmniRouteModel, OmniRouteModelsResponse } from './types.js'; import { OMNIROUTE_DEFAULT_MODELS, OMNIROUTE_ENDPOINTS, MODEL_CACHE_TTL, REQUEST_TIMEOUT, } from './constants.js'; -import { getModelsDevIndex, normalizeModelKey } from './models-dev.js'; -import type { ModelsDevIndex } from './models-dev.js'; -import { enrichComboModels, clearComboCache } from './omniroute-combos.js'; +import { + getModelsDevIndex, + normalizeModelKey, + getSubscriptionFallback, + stripVariantSuffix, + resolveProviderAlias, + resolveModelAlias, +} from './models-dev.js'; +import type { ModelsDevIndex, ModelsDevModel } from './models-dev.js'; +import { enrichComboModels, clearComboCache, splitModelId } from './omniroute-combos.js'; import { warn, debug } from './logger.js'; /** @@ -28,7 +35,17 @@ const modelCache = new Map(); */ function getCacheKey(config: OmniRouteConfig, apiKey: string): string { const baseUrl = config.baseUrl || OMNIROUTE_ENDPOINTS.BASE_URL; - return `${baseUrl}:${apiKey}`; + + // Include modelsDev config in cache key to prevent stale data + const modelsDevHash = config.modelsDev + ? JSON.stringify({ + enabled: config.modelsDev.enabled, + url: config.modelsDev.url, + providerAliases: config.modelsDev.providerAliases, + }) + : ''; + + return `${baseUrl}:${apiKey}:${modelsDevHash}`; } /** @@ -116,6 +133,9 @@ export async function fetchModels( supportsStreaming: model.supportsStreaming, supportsVision: model.supportsVision, supportsTools: model.supportsTools, + supportsTemperature: model.supportsTemperature, + supportsReasoning: model.supportsReasoning, + supportsAttachment: model.supportsAttachment, })); // Enrich with models.dev and combo capabilities @@ -234,32 +254,17 @@ function applyModelsDevMetadata( config: OmniRouteConfig, index: ModelsDevIndex, ): OmniRouteModel { - const { providerKey, modelKey } = splitOmniRouteModelForLookup(model.id); + const { providerKey, modelKey } = splitModelId(model.id); const providerAlias = resolveProviderAlias(providerKey, config); - const lookupKey = modelKey.toLowerCase(); - const normalizedKey = normalizeModelKey(modelKey); - - // Try provider-specific exact match first - const providerExact = providerAlias - ? index.exactByProvider.get(providerAlias)?.get(lookupKey) - : undefined; - - // Try provider-specific normalized match - const providerNorm = providerAlias - ? index.normalizedByProvider.get(providerAlias)?.get(normalizedKey) - : undefined; - - // Try global exact match (only if single match to avoid ambiguity) - const globalExactList = index.exactGlobal.get(lookupKey); - const globalExact = globalExactList?.length === 1 ? globalExactList[0] : undefined; - - // Try global normalized match (only if single match to avoid ambiguity) - const globalNormList = index.normalizedGlobal.get(normalizedKey); - const globalNorm = globalNormList?.length === 1 ? globalNormList[0] : undefined; - - // Pick the best match (provider-specific preferred over global) - const best = providerExact ?? providerNorm ?? globalExact ?? globalNorm; - + const candidates = getModelLookupCandidates(modelKey); + const providerCandidates = [ + ...(providerAlias ? [providerAlias] : []), + ...(providerAlias + ? [getSubscriptionFallback(providerAlias)].filter((p): p is string => p !== null) + : []), + ]; + + const best = lookupModelsDevModel(index, providerCandidates, candidates); if (!best) return model; // Merge capabilities (only fill in missing values) @@ -278,66 +283,65 @@ function applyModelsDevMetadata( ? { supportsTools: true } : {}), ...(model.supportsStreaming === undefined - ? { supportsStreaming: true } // Assume streaming is supported by default + ? { supportsStreaming: true } + : {}), + ...(model.supportsTemperature === undefined && best.temperature !== undefined + ? { supportsTemperature: best.temperature } + : {}), + ...(model.supportsReasoning === undefined && best.reasoning !== undefined + ? { supportsReasoning: best.reasoning } + : {}), + ...(model.supportsAttachment === undefined && best.attachment !== undefined + ? { supportsAttachment: best.attachment } : {}), }; } -/** - * Split model ID for models.dev lookup - */ -function splitOmniRouteModelForLookup( - modelId: string, -): { providerKey: string | null; modelKey: string } { - const trimmed = modelId.trim(); - - // Remove omniroute prefix if present - const withoutPrefix = trimmed.replace(/^omniroute\//, ''); - - // Split by / - const parts = withoutPrefix.split('/').filter((p) => p.trim() !== ''); - - if (parts.length >= 2) { - return { - providerKey: parts[0] ?? null, - modelKey: parts.slice(1).join('/'), - }; +function getModelLookupCandidates(modelKey: string): string[] { + const candidates = new Set(); + const addCandidate = (key: string): void => { + candidates.add(key.toLowerCase()); + candidates.add(resolveModelAlias(key).toLowerCase()); + candidates.add(normalizeModelKey(key)); + candidates.add(normalizeModelKey(resolveModelAlias(key))); + }; + + addCandidate(modelKey); + + const { base, stripped } = stripVariantSuffix(modelKey); + if (stripped) { + addCandidate(base); } - return { providerKey: null, modelKey: withoutPrefix }; + return [...candidates]; } -/** - * Resolve provider alias using config - */ -function resolveProviderAlias( - providerKey: string | null, - config: OmniRouteConfig, -): string | null { - if (!providerKey) return null; - - const lower = providerKey.toLowerCase(); - - // Default aliases - const aliases: Record = { - oai: 'openai', - openai: 'openai', - cx: 'openai', - codex: 'openai', - anthropic: 'anthropic', - claude: 'anthropic', - gemini: 'google', - google: 'google', - deepseek: 'deepseek', - mistral: 'mistral', - xai: 'xai', - groq: 'groq', - together: 'together', - openrouter: 'openrouter', - perplexity: 'perplexity', - cohere: 'cohere', - ...config.modelsDev?.providerAliases, - }; +function lookupModelsDevModel( + index: ModelsDevIndex, + providerCandidates: string[], + modelCandidates: string[], +): ModelsDevModel | undefined { + for (const provider of providerCandidates) { + for (const candidate of modelCandidates) { + const exact = index.exactByProvider.get(provider)?.get(candidate); + if (exact) return exact; + + const normalized = index.normalizedByProvider + .get(provider) + ?.get(normalizeModelKey(candidate)); + if (normalized) return normalized; + } + } - return aliases[lower] ?? lower; + for (const candidate of modelCandidates) { + const exactList = index.exactGlobal.get(candidate); + if (exactList?.length === 1) return exactList[0]; + + const normalizedList = index.normalizedGlobal.get(normalizeModelKey(candidate)); + if (normalizedList?.length === 1) return normalizedList[0]; + } + + return undefined; } + + diff --git a/src/omniroute-combos.ts b/src/omniroute-combos.ts index 1650003..ccc6b3d 100644 --- a/src/omniroute-combos.ts +++ b/src/omniroute-combos.ts @@ -224,7 +224,7 @@ export function lookupModelInIndex( * Split a model ID into provider and model key * Handles formats like "provider/model", "omniroute/provider/model", etc. */ -function splitModelId(modelId: string): { providerKey: string | null; modelKey: string } { +export function splitModelId(modelId: string): { providerKey: string | null; modelKey: string } { const trimmed = modelId.trim(); // Remove omniroute prefix if present @@ -366,6 +366,15 @@ export async function enrichComboModels( ...(capabilities.maxTokens !== undefined ? { maxTokens: capabilities.maxTokens } : {}), ...(capabilities.supportsVision !== undefined ? { supportsVision: capabilities.supportsVision } : {}), ...(capabilities.supportsTools !== undefined ? { supportsTools: capabilities.supportsTools } : {}), + ...(capabilities.supportsTemperature !== undefined + ? { supportsTemperature: capabilities.supportsTemperature } + : {}), + ...(capabilities.supportsReasoning !== undefined + ? { supportsReasoning: capabilities.supportsReasoning } + : {}), + ...(capabilities.supportsAttachment !== undefined + ? { supportsAttachment: capabilities.supportsAttachment } + : {}), ...(capabilities.supportsStreaming !== undefined ? { supportsStreaming: capabilities.supportsStreaming } : {}), ...(capabilities.pricing !== undefined ? { pricing: { ...model.pricing, ...capabilities.pricing } } : {}), }; diff --git a/src/plugin.ts b/src/plugin.ts index e695e61..c709fa3 100644 --- a/src/plugin.ts +++ b/src/plugin.ts @@ -6,6 +6,7 @@ import type { OmniRouteApiMode, OmniRouteConfig, OmniRouteModel, + OmniRouteModelMetadata, OmniRouteModelMetadataConfig, OmniRouteModelsDevConfig, OmniRouteProviderModel, @@ -41,14 +42,35 @@ export const OmniRouteAuthPlugin: Plugin = async (_input) => { let models: OmniRouteModel[] = OMNIROUTE_DEFAULT_MODELS; try { const auth = await readAuthFromStore(OMNIROUTE_PROVIDER_ID); - if (auth?.key) { - const runtimeConfig = createRuntimeConfig(existingProvider?.options ?? {}, auth.key); - models = await fetchModels(runtimeConfig, auth.key, false); + const apiKey = auth?.key ?? process.env.OMNIROUTE_API_KEY; + if (apiKey) { + const runtimeConfig = createRuntimeConfig(existingProvider?.options ?? {}, apiKey); + models = await fetchModels(runtimeConfig, apiKey, false); } } catch (error) { warn(`Eager model fetch failed, using defaults: ${error}`); } + const generatedModelMetadata: Record = {}; + for (const model of models) { + generatedModelMetadata[model.id] = { + contextWindow: model.contextWindow, + maxTokens: model.maxTokens, + supportsTemperature: model.supportsTemperature, + supportsReasoning: model.supportsReasoning, + supportsAttachment: model.supportsAttachment, + supportsVision: model.supportsVision, + supportsTools: model.supportsTools, + supportsStreaming: model.supportsStreaming, + pricing: model.pricing, + }; + } + + const modelMetadata = mergeModelMetadata( + existingProvider?.options?.modelMetadata, + generatedModelMetadata, + ); + providers[OMNIROUTE_PROVIDER_ID] = { ...existingProvider, name: existingProvider?.name ?? OMNIROUTE_PROVIDER_NAME, @@ -59,6 +81,7 @@ export const OmniRouteAuthPlugin: Plugin = async (_input) => { ...(existingProvider?.options ?? {}), baseURL: baseUrl, apiMode, + modelMetadata, }, models: existingProvider?.models && Object.keys(existingProvider.models).length > 0 @@ -325,6 +348,40 @@ function getStringRecord(value: unknown): Record | undefined { return Object.keys(out).length > 0 ? out : undefined; } +function mergeModelMetadata( + rawUserConfig: unknown, + generated: Record, +): OmniRouteModelMetadataConfig { + const userConfig = getModelMetadataConfig({ modelMetadata: rawUserConfig }); + + if (Array.isArray(userConfig)) { + const generatedBlocks = Object.entries(generated).map(([id, metadata]) => ({ + match: id, + ...metadata, + })); + + return [...generatedBlocks, ...userConfig]; + } + + if (userConfig && isRecord(userConfig)) { + const merged: Record = { ...generated }; + for (const [id, metadata] of Object.entries(userConfig)) { + const validation = isValidModelMetadata(metadata); + if (!validation.valid) { + warn(`Invalid metadata for model "${id}" (field: ${validation.field}), skipping`); + continue; + } + merged[id] = { + ...(generated[id] ?? {}), + ...metadata, + }; + } + return merged; + } + + return generated; +} + function isRegExp(value: unknown): value is RegExp { return Object.prototype.toString.call(value) === '[object RegExp]'; } @@ -363,6 +420,48 @@ function isRecord(value: unknown): value is Record { return typeof value === 'object' && value !== null && !Array.isArray(value); } +const BOOLEAN_FIELDS = [ + 'supportsStreaming', 'supportsVision', 'supportsTools', + 'supportsTemperature', 'supportsReasoning', 'supportsAttachment', +]; + +function isValidModelMetadata(value: unknown): { valid: boolean; field?: string } { + if (!isRecord(value)) return { valid: false, field: '(not an object)' }; + + for (const field of BOOLEAN_FIELDS) { + if (field in value && typeof value[field] !== 'boolean') { + return { valid: false, field }; + } + } + + if ('contextWindow' in value && typeof value.contextWindow !== 'number') { + return { valid: false, field: 'contextWindow' }; + } + if ('maxTokens' in value && typeof value.maxTokens !== 'number') { + return { valid: false, field: 'maxTokens' }; + } + if ('name' in value && typeof value.name !== 'string') { + return { valid: false, field: 'name' }; + } + if ('description' in value && typeof value.description !== 'string') { + return { valid: false, field: 'description' }; + } + if ('pricing' in value) { + const pricing = value.pricing; + if (!isRecord(pricing)) { + return { valid: false, field: 'pricing' }; + } + if ('input' in pricing && typeof pricing.input !== 'number') { + return { valid: false, field: 'pricing.input' }; + } + if ('output' in pricing && typeof pricing.output !== 'number') { + return { valid: false, field: 'pricing.output' }; + } + } + + return { valid: true }; +} + function toProviderModels( models: OmniRouteModel[], baseUrl: string, @@ -377,6 +476,9 @@ function toProviderModels( function toProviderModel(model: OmniRouteModel, baseUrl: string): OmniRouteProviderModel { const supportsVision = model.supportsVision === true; const supportsTools = model.supportsTools !== false; + const supportsTemperature = model.supportsTemperature !== false; + const supportsReasoning = model.supportsReasoning === true; + const supportsAttachment = model.supportsAttachment !== undefined ? model.supportsAttachment : supportsVision; return { id: model.id, @@ -384,15 +486,23 @@ function toProviderModel(model: OmniRouteModel, baseUrl: string): OmniRouteProvi providerID: OMNIROUTE_PROVIDER_ID, family: getModelFamily(model.id), release_date: '', + attachment: supportsAttachment, + reasoning: supportsReasoning, + temperature: supportsTemperature, + tool_call: supportsTools, + modalities: { + input: supportsVision ? ['text', 'image'] as const : ['text'] as const, + output: ['text'] as const, + }, api: { id: model.id, url: baseUrl, npm: OMNIROUTE_PROVIDER_NPM, }, capabilities: { - temperature: true, - reasoning: false, - attachment: supportsVision, + temperature: supportsTemperature, + reasoning: supportsReasoning, + attachment: supportsAttachment, toolcall: supportsTools, input: { text: true, @@ -425,7 +535,13 @@ function toProviderModel(model: OmniRouteModel, baseUrl: string): OmniRouteProvi options: {}, headers: {}, status: 'active', - variants: {}, + variants: supportsReasoning + ? { + low: { reasoningEffort: 'low' }, + medium: { reasoningEffort: 'medium' }, + high: { reasoningEffort: 'high' }, + } + : {}, }; } diff --git a/src/types.ts b/src/types.ts index 11d359a..88022e9 100644 --- a/src/types.ts +++ b/src/types.ts @@ -10,6 +10,9 @@ export interface OmniRouteModel { supportsStreaming?: boolean; supportsVision?: boolean; supportsTools?: boolean; + supportsTemperature?: boolean; + supportsReasoning?: boolean; + supportsAttachment?: boolean; pricing?: { input?: number; output?: number; @@ -24,6 +27,9 @@ export interface OmniRouteModelMetadata { supportsStreaming?: boolean; supportsVision?: boolean; supportsTools?: boolean; + supportsTemperature?: boolean; + supportsReasoning?: boolean; + supportsAttachment?: boolean; pricing?: { input?: number; output?: number; @@ -108,6 +114,14 @@ export interface OmniRouteProviderModel { providerID: string; family: string; release_date: string; + attachment?: boolean; + reasoning?: boolean; + temperature?: boolean; + tool_call?: boolean; + modalities?: { + input: readonly string[]; + output: readonly string[]; + }; api: { id: string; url: string; diff --git a/test/models.test.mjs b/test/models.test.mjs index 70137f8..81b80aa 100644 --- a/test/models.test.mjs +++ b/test/models.test.mjs @@ -8,6 +8,7 @@ import { isCacheValid, refreshModels, } from '../dist/runtime.js'; +import { calculateLowestCommonCapabilities } from '../dist/src/models-dev.js'; const ORIGINAL_FETCH = global.fetch; @@ -102,3 +103,44 @@ test('fetchModels falls back to defaults when response shape is invalid', async assert.ok(models.length > 0); assert.ok(typeof models[0].id === 'string'); }); + +test('calculateLowestCommonCapabilities ignores missing attachment metadata', () => { + const capabilities = calculateLowestCommonCapabilities([ + { id: 'with-attachment', attachment: true }, + { id: 'without-attachment' }, + ]); + + assert.equal(capabilities.supportsAttachment, true); +}); + +test('calculateLowestCommonCapabilities respects explicit attachment false', () => { + const capabilities = calculateLowestCommonCapabilities([ + { id: 'with-attachment', attachment: true }, + { id: 'without-attachment', attachment: false }, + ]); + + assert.equal(capabilities.supportsAttachment, false); +}); + +test('fetchModels uses different cache for different modelsDev configs', async () => { + let calls = 0; + global.fetch = async (input) => { + const url = input instanceof Request ? input.url : input.toString(); + if (url.includes('/v1/models')) { + calls++; + return new Response( + JSON.stringify({ object: 'list', data: [{ id: `model-${calls}`, name: `Model ${calls}` }] }), + { status: 200, headers: { 'Content-Type': 'application/json' } } + ); + } + return new Response(JSON.stringify({ data: [] }), { status: 200 }); + }; + + const config1 = { ...CONFIG, modelsDev: { enabled: true, providerAliases: { oai: 'openai' } } }; + const config2 = { ...CONFIG, modelsDev: { enabled: true, providerAliases: { oai: 'anthropic' } } }; + + await fetchModels(config1, CONFIG.apiKey, false); + await fetchModels(config2, CONFIG.apiKey, false); + + assert.equal(calls, 2, 'Should fetch twice for different modelsDev configs'); +}); diff --git a/test/plugin.test.mjs b/test/plugin.test.mjs index 7c20a9e..35cd383 100644 --- a/test/plugin.test.mjs +++ b/test/plugin.test.mjs @@ -433,3 +433,174 @@ test('config hook eagerly fetches models when auth is available', async () => { await rm(tempHome, { recursive: true, force: true }); } }); + +test('config hook preserves user modelMetadata object overrides', async () => { + const tempHome = join(tmpdir(), `opencode-test-${Date.now()}`); + try { + await mkdir(join(tempHome, '.local', 'share', 'opencode'), { recursive: true }); + await writeFile( + join(tempHome, '.local', 'share', 'opencode', 'auth.json'), + JSON.stringify({ omniroute: { type: 'api', key: 'test-key' } }), + ); + process.env.HOME = tempHome; + + global.fetch = async (input) => { + const url = input instanceof Request ? input.url : String(input); + if (url.endsWith('/v1/models')) { + return new Response( + JSON.stringify({ + object: 'list', + data: [ + { + id: 'cx/gpt-5.5', + name: 'GPT-5.5', + contextWindow: 1050000, + supportsReasoning: true, + }, + ], + }), + { status: 200, headers: { 'Content-Type': 'application/json' } }, + ); + } + return new Response(JSON.stringify({ ok: true }), { + status: 200, + headers: { 'Content-Type': 'application/json' }, + }); + }; + + const plugin = await OmniRouteAuthPlugin({}); + const config = { + provider: { + omniroute: { + options: { + baseURL: 'http://localhost:20129/v1', + modelMetadata: { + 'cx/gpt-5.5': { + contextWindow: 258000, + }, + }, + }, + }, + }, + }; + + await plugin.config(config); + + const metadata = config.provider.omniroute.options.modelMetadata['cx/gpt-5.5']; + assert.equal(metadata.contextWindow, 258000); + assert.equal(metadata.supportsReasoning, true); + } finally { + await rm(tempHome, { recursive: true, force: true }); + } +}); + +test('config hook preserves user modelMetadata match blocks', async () => { + const tempHome = join(tmpdir(), `opencode-test-${Date.now()}`); + try { + await mkdir(join(tempHome, '.local', 'share', 'opencode'), { recursive: true }); + await writeFile( + join(tempHome, '.local', 'share', 'opencode', 'auth.json'), + JSON.stringify({ omniroute: { type: 'api', key: 'test-key' } }), + ); + process.env.HOME = tempHome; + + global.fetch = async (input) => { + const url = input instanceof Request ? input.url : String(input); + if (url.endsWith('/v1/models')) { + return new Response( + JSON.stringify({ + object: 'list', + data: [{ id: 'cx/gpt-5.5', name: 'GPT-5.5', contextWindow: 1050000 }], + }), + { status: 200, headers: { 'Content-Type': 'application/json' } }, + ); + } + return new Response(JSON.stringify({ ok: true }), { + status: 200, + headers: { 'Content-Type': 'application/json' }, + }); + }; + + const userBlock = { + match: '^(codex|cx)/.*gpt-5', + contextWindow: 258000, + }; + const plugin = await OmniRouteAuthPlugin({}); + const config = { + provider: { + omniroute: { + options: { + baseURL: 'http://localhost:20130/v1', + modelMetadata: [userBlock], + }, + }, + }, + }; + + await plugin.config(config); + + const metadata = config.provider.omniroute.options.modelMetadata; + assert.ok(Array.isArray(metadata)); + assert.deepEqual(metadata.at(-1), userBlock); + assert.equal(metadata[0].match, 'cx/gpt-5.5'); + assert.equal(metadata[0].contextWindow, 1050000); + } finally { + await rm(tempHome, { recursive: true, force: true }); + } +}); + +test('config hook respects explicit attachment false for vision models', async () => { + const tempHome = join(tmpdir(), `opencode-test-${Date.now()}`); + try { + await mkdir(join(tempHome, '.local', 'share', 'opencode'), { recursive: true }); + await writeFile( + join(tempHome, '.local', 'share', 'opencode', 'auth.json'), + JSON.stringify({ omniroute: { type: 'api', key: 'test-key' } }), + ); + process.env.HOME = tempHome; + + global.fetch = async (input) => { + const url = input instanceof Request ? input.url : String(input); + if (url.endsWith('/v1/models')) { + return new Response( + JSON.stringify({ + object: 'list', + data: [ + { + id: 'vision-no-attachment', + name: 'Vision No Attachment', + supportsVision: true, + supportsAttachment: false, + }, + ], + }), + { status: 200, headers: { 'Content-Type': 'application/json' } }, + ); + } + return new Response(JSON.stringify({ ok: true }), { + status: 200, + headers: { 'Content-Type': 'application/json' }, + }); + }; + + const plugin = await OmniRouteAuthPlugin({}); + const config = { + provider: { + omniroute: { + options: { + baseURL: 'http://localhost:20131/v1', + }, + }, + }, + }; + + await plugin.config(config); + + const model = config.provider.omniroute.models['vision-no-attachment']; + assert.equal(model.attachment, false); + assert.equal(model.capabilities.attachment, false); + assert.deepEqual(model.modalities.input, ['text', 'image']); + } finally { + await rm(tempHome, { recursive: true, force: true }); + } +});