feat(models-info): add @vymalo/opencode-models-info plugin#18
Conversation
Add a second OpenCode plugin that enriches existing model entries with full metadata (context length, output limit, USD/M-token cost, modalities, tool_call/reasoning/attachment flags) by fetching from a provider-supplied OpenRouter-shaped endpoint declared via `options.meta.modelsInfoUrl`. Auth-agnostic by design: runs as a Hooks.config hook after other plugins have populated providers and headers, so it composes with @vymalo/opencode-oauth2, static API keys, or any other auth scheme without depending on any of them. Upstream values are never overwritten. Includes a TTL'd two-layer cache (in-memory + disk, atomic writes, ETag honored) mirroring the oauth2 cache pattern, with stale-on-failure fallback so a flaky endpoint never blocks OpenCode startup. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces the @vymalo/opencode-models-info plugin, which enriches OpenCode model entries with metadata fetched from an OpenRouter-shaped endpoint. It features a file-based cache store, robust mapping logic, and a config hook to process providers in parallel. The review feedback highlights critical improvements to ensure the plugin is truly auth-agnostic by extracting and forwarding the provider's configured headers, merging them with specific metadata headers. Additionally, the reviewer suggests wrapping the provider enrichment process in a try-catch block to prevent errors from being silently swallowed during parallel execution, and points out a potential path duplication bug in the relative URL resolution logic.
| async function enrichProvider( | ||
| providerId: string, | ||
| providerConfig: ProviderConfigLike | undefined, | ||
| deps: EnrichDeps | ||
| ): Promise<void> { | ||
| if (!providerConfig) { | ||
| return; | ||
| } | ||
| const opts = parseMetaOptions(providerConfig.options); | ||
| if (!opts) { | ||
| return; | ||
| } | ||
| const models = providerConfig.models; | ||
| if (!models || Object.keys(models).length === 0) { | ||
| deps.logger.debug("models_info_provider_skipped_no_models", { providerId }); | ||
| return; | ||
| } | ||
|
|
||
| const record = await loadRecord(providerId, opts, deps); | ||
| if (!record) { | ||
| return; | ||
| } | ||
|
|
||
| const byId = new Map<string, OpenRouterModel>(record.models.map((m) => [m.id, m])); | ||
|
|
||
| let enrichedCount = 0; | ||
| for (const [modelId, modelConfig] of Object.entries(models)) { | ||
| const declaredId = typeof modelConfig.id === "string" ? modelConfig.id : undefined; | ||
| const match = byId.get(modelId) ?? (declaredId ? byId.get(declaredId) : undefined); | ||
| if (!match) { | ||
| continue; | ||
| } | ||
| const derived = mapOpenRouterEntry(match); | ||
| mergeIntoModel(modelConfig, derived); | ||
| enrichedCount += 1; | ||
| } | ||
|
|
||
| deps.logger.info("models_info_enriched", { | ||
| providerId, | ||
| enrichedCount, | ||
| totalModels: Object.keys(models).length, | ||
| sourceModels: record.models.length | ||
| }); | ||
| } |
There was a problem hiding this comment.
To satisfy the design goal of being auth-agnostic and reusing the provider's resolved authentication headers (e.g., from @vymalo/opencode-oauth2 or static API keys), we need to extract and pass the provider's configured headers to loadRecord so they can be forwarded in the fetch request. Additionally, since enrichConfig runs all providers in parallel using Promise.allSettled and does not inspect or log the rejected promises, any unexpected error thrown inside enrichProvider (such as a disk write failure in FileCacheStore.put or a network/parsing error) will be silently swallowed without any log output. Wrapping the body of enrichProvider in a try-catch block ensures that any failure is logged via deps.logger.error.
async function enrichProvider(
providerId: string,
providerConfig: ProviderConfigLike | undefined,
deps: EnrichDeps
): Promise<void> {
try {
if (!providerConfig) {
return;
}
const opts = parseMetaOptions(providerConfig.options);
if (!opts) {
return;
}
const models = providerConfig.models;
if (!models || Object.keys(models).length === 0) {
deps.logger.debug("models_info_provider_skipped_no_models", { providerId });
return;
}
const providerHeaders = providerConfig.options?.headers as Record<string, string> | undefined;
const record = await loadRecord(providerId, opts, providerHeaders, deps);
if (!record) {
return;
}
const byId = new Map<string, OpenRouterModel>(record.models.map((m) => [m.id, m]));
let enrichedCount = 0;
for (const [modelId, modelConfig] of Object.entries(models)) {
const declaredId = typeof modelConfig.id === "string" ? modelConfig.id : undefined;
const match = byId.get(modelId) ?? (declaredId ? byId.get(declaredId) : undefined);
if (!match) {
continue;
}
const derived = mapOpenRouterEntry(match);
mergeIntoModel(modelConfig, derived);
enrichedCount += 1;
}
deps.logger.info("models_info_enriched", {
providerId,
enrichedCount,
totalModels: Object.keys(models).length,
sourceModels: record.models.length
});
} catch (error) {
deps.logger.error("models_info_enrichment_failed", {
providerId,
error: error instanceof Error ? error.message : String(error)
});
}
}| async function loadRecord( | ||
| providerId: string, | ||
| opts: MetaProviderOptions, | ||
| deps: EnrichDeps | ||
| ): Promise<CachedModelsRecord | undefined> { | ||
| const key = cacheKey(providerId, opts.modelsInfoUrl); | ||
| const now = deps.now ? deps.now() : Date.now(); | ||
| const cached = await deps.cache.get(key); | ||
|
|
||
| if (cached && !isExpired(cached, now)) { | ||
| deps.logger.debug("models_info_cache_hit", { | ||
| providerId, | ||
| url: opts.modelsInfoUrl, | ||
| ageMs: now - cached.fetchedAt | ||
| }); | ||
| return cached; | ||
| } | ||
|
|
||
| const headers = buildFetchHeaders(opts); |
There was a problem hiding this comment.
Update loadRecord signature to accept the provider's resolved headers and forward them to buildFetchHeaders.
async function loadRecord(
providerId: string,
opts: MetaProviderOptions,
providerHeaders: Record<string, string> | undefined,
deps: EnrichDeps
): Promise<CachedModelsRecord | undefined> {
const key = cacheKey(providerId, opts.modelsInfoUrl);
const now = deps.now ? deps.now() : Date.now();
const cached = await deps.cache.get(key);
if (cached && !isExpired(cached, now)) {
deps.logger.debug("models_info_cache_hit", {
providerId,
url: opts.modelsInfoUrl,
ageMs: now - cached.fetchedAt
});
return cached;
}
const headers = buildFetchHeaders(opts, providerHeaders);| function buildFetchHeaders(opts: MetaProviderOptions): Record<string, string> | undefined { | ||
| return opts.modelsInfoHeaders; | ||
| } |
There was a problem hiding this comment.
Merge the provider's general headers with the specific modelsInfoHeaders (with modelsInfoHeaders taking precedence) to ensure authenticated requests work correctly.
| function buildFetchHeaders(opts: MetaProviderOptions): Record<string, string> | undefined { | |
| return opts.modelsInfoHeaders; | |
| } | |
| function buildFetchHeaders( | |
| opts: MetaProviderOptions, | |
| providerHeaders: Record<string, string> | undefined | |
| ): Record<string, string> | undefined { | |
| if (!providerHeaders && !opts.modelsInfoHeaders) { | |
| return undefined; | |
| } | |
| return { | |
| ...(providerHeaders ?? {}), | |
| ...(opts.modelsInfoHeaders ?? {}) | |
| }; | |
| } |
| return candidate; | ||
| } | ||
| const base = baseURL.endsWith("/") ? baseURL : `${baseURL}/`; | ||
| const rel = candidate.startsWith("/") ? candidate.slice(1) : candidate; |
There was a problem hiding this comment.
Stripping the leading slash from candidate to resolve it relative to the baseURL path works for /models/info, but it will cause duplication of path segments if the candidate already includes the base path (e.g., if baseURL is https://example.com/v1 and modelsInfoUrl is /v1/models/info, it resolves to https://example.com/v1/v1/models/info). This contradicts the plan's suggestion that authors can write "modelsInfoUrl": "/v1/models/info".
Consider checking if the candidate already starts with the base URL's path segment before stripping, or standardizing on standard URL resolution where leading slashes resolve from the host root.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1918e0c5a0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| function buildFetchHeaders(opts: MetaProviderOptions): Record<string, string> | undefined { | ||
| return opts.modelsInfoHeaders; | ||
| } |
There was a problem hiding this comment.
The request headers are built from meta.modelsInfoHeaders only, so existing provider headers (for example provider.options.headers.Authorization or API-key headers) are dropped before calling fetchOpenRouterModels. In authenticated deployments this causes /models/info calls to fail (typically 401/403), which prevents enrichment and contradicts the plugin’s auth-agnostic behavior for providers that already have working request headers.
Useful? React with 👍 / 👎.
| const refreshed: CachedModelsRecord = { ...cached, fetchedAt: now }; | ||
| await deps.cache.put(key, refreshed); |
There was a problem hiding this comment.
When a cached entry is revalidated with 304 Not Modified, the code copies the old record unchanged except for fetchedAt, so ttlSeconds remains whatever value was stored previously. If meta.modelsInfoTtlSeconds is changed in config, that new TTL is ignored until a future 200 response, causing cache freshness behavior to diverge from user configuration.
Useful? React with 👍 / 👎.
| return body.filter(isOpenRouterModel); | ||
| } | ||
| if (body && typeof body === "object") { | ||
| const data = (body as OpenRouterModelsResponse).data; | ||
| if (Array.isArray(data)) { | ||
| return data.filter(isOpenRouterModel); | ||
| } |
There was a problem hiding this comment.
normalizeResponse silently filters out invalid entries and still returns success, so a malformed data array (for example, items missing string id) can become [] and be cached as a fresh snapshot. That overwrites previously valid cached metadata and disables enrichment until TTL expiry, instead of falling back to stale data on a parse error.
Useful? React with 👍 / 👎.
Unit tests: - test/config.test.ts — parseMetaOptions edge cases (relative URL resolution, header coercion, defaulting, invalid inputs) - test/fetcher.test.ts — fetchOpenRouterModels (envelope vs bare-array, ETag round-trip, header merging, timeout via AbortController, non-throwing error results) Test env (reusable across the workspace): - test-env/docker-compose.yml — WireMock service with healthcheck on port 18080 (Keycloak placeholder commented in, ready for the oauth2 integration suite later). - test-env/wiremock/ — OpenRouter-shaped catalog fixture + mappings covering happy-path 200, ETag-aware 304, and a Bearer-required ?auth=required variant for header-propagation tests. - test-env/README.md — quick start, endpoints, stub-editing notes. Integration suite: - packages/opencode-models-info/test/integration/*.integration.test.ts — runs against the live WireMock; verifies enrichment, disk cache + 304 path, header propagation, and 401-on-missing-auth fallback. - Separate vitest.integration.config.ts so the default `pnpm test` stays hermetic and fast. - Tests skip themselves when INTEGRATION_MODELS_INFO_URL is unset. Scripts: - Root: test:env:up / test:env:down (with --wait healthcheck gating) and test:integration which orchestrates up → all packages' integration suites (via `pnpm -r --if-present`) → down. - Package: test:integration runs the integration vitest config. Docs: - CLAUDE.md — new repo-wide guide for future Claude Code sessions, including the integration env section. - packages/opencode-models-info/README.md — Testing section with both unit and integration recipes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 68daa86d74
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| etag: result.etag, | ||
| models: result.models | ||
| }; | ||
| await deps.cache.put(key, next); |
There was a problem hiding this comment.
In environments where the cache directory is read-only or cannot be created (for example a locked-down container with an unwritable $HOME/XDG_CACHE_HOME), FileCacheStore.put throws here after a successful fetch, causing enrichProvider to reject before returning next; Promise.allSettled then silently drops the provider and no models are enriched even though the network response was usable. The cache should be best-effort, so a failed write should be logged/ignored while still using the freshly fetched record.
Useful? React with 👍 / 👎.
| opts: MetaProviderOptions, | ||
| deps: EnrichDeps | ||
| ): Promise<CachedModelsRecord | undefined> { | ||
| const key = cacheKey(providerId, opts.modelsInfoUrl); |
There was a problem hiding this comment.
When meta.modelsInfoHeaders changes for the same provider and URL (for example switching an x-tenant header or adding an auth header that exposes a different catalog), this cache key still hits the old snapshot and skips the fetch until the previous TTL expires, so model metadata from the prior header context can be applied to the new configuration for up to the default 24 hours. Either the cache key or cached record needs to account for headers that can vary the response.
Useful? React with 👍 / 👎.
Extend the existing publish workflow to also build, pack, verify, and publish @vymalo/opencode-models-info alongside the oauth2 plugin. - Tarball verification mirrors the oauth2 checks (dist/index.js present, src/test/node_modules absent, LICENSE present). - Each package's publish step is now guarded by an `npm view` check — if the version in package.json is already on the registry, the step logs a notice and exits 0 instead of failing the whole workflow. Means a release that only bumps one package's version no longer fails on the unchanged one. - Release-attachment glob updated to `dist-tarball/**/*.tgz` so both packages' tarballs are uploaded. Also pick up biome format --write auto-fixes across the new package's source and tests — purely cosmetic, makes `pnpm format:check` green so the publish workflow's format-check gate stays clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 66dfb841c4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const params = entry.supported_parameters ?? []; | ||
| const paramSet = new Set(params.map((p) => p.toLowerCase())); |
There was a problem hiding this comment.
When a fetched catalog entry has a string or otherwise non-array supported_parameters field, normalizeResponse still accepts it because it only checks id, and this .map() then throws. In that scenario one malformed model from an external models-info endpoint makes enrichProvider reject and Promise.allSettled silently leaves the whole provider un-enriched, including any valid models in the same response; coerce with Array.isArray or validate this field before caching/mapping.
Useful? React with 👍 / 👎.
| if (inputMods.some((m) => m !== "text")) { | ||
| out.attachment = true; |
There was a problem hiding this comment.
When the provider advertises a non-text modality that OpenCode cannot put in modalities.input (for example OpenRouter's file), filterModalities drops it before this attachment check runs, so a model with input_modalities: ["text", "file"] is left without attachment: true. That makes file-capable models look text-only to OpenCode even though the response explicitly advertised attachment support; derive the attachment flag from the original input modalities before filtering.
Useful? React with 👍 / 👎.
| deps.logger.info("models_info_fetched", { | ||
| providerId, | ||
| url: opts.modelsInfoUrl, | ||
| count: result.models.length |
There was a problem hiding this comment.
When meta.modelsInfoUrl is an absolute URL that carries credentials in userinfo or query parameters (for example a signed URL or ?api_key=...), this logs the raw URL to both console fallback and client.app.log; the existing redaction only keys off field names and url is not redacted. In that configuration a routine successful fetch exposes the secret in OpenCode logs, so log a sanitized URL or omit query/userinfo.
Useful? React with 👍 / 👎.
Addresses inline review comments from gemini-code-assist and codex on PR #18. H1 — buildFetchHeaders now merges provider.options.headers ⨁ meta.modelsInfoHeaders (meta wins on conflict). Restores the auth-agnostic claim: static API keys and any plugin that has already populated provider.options.headers (e.g. @vymalo/opencode-oauth2 ≥ 0.4.0) flow through naturally without coupling. H2 — enrichProvider wrapped in try/catch. Promise.allSettled was silently swallowing any throw (disk write failures, mapping bugs); now surfaced via deps.logger.error("models_info_enrichment_failed"). M1 — resolveUrl uses standard WHATWG URL semantics. Previously stripped the leading slash from the candidate before resolving, which produced `https://x.test/v1/v1/models` when a user wrote `modelsInfoUrl: "/v1/models"` under `baseURL: "https://x.test/v1"`. Now: - `modelsInfoUrl: "models/info"` joins under baseURL's path (recommended). - `modelsInfoUrl: "/models/info"` is origin-rooted (escape hatch for metadata endpoints at a different path than the inference API). README and JSDoc updated; the integration test fixture was already absolute-URL based so untouched. M2 — 304 path applies the current TTL from config instead of carrying over the stored ttlSeconds. A tightened TTL takes effect on the next revalidation, not on the next full 200. M3 — fetcher treats "non-empty input filtered down to empty" as a parse error. A malformed catalog response (entries missing string `id`) no longer overwrites previously good cached data with []; the stale-cache fallback kicks in instead. M4 — cache.put is now best-effort. A read-only $HOME / cache dir previously made the fetched record evaporate; now we log `models_info_cache_write_failed` and still return the in-memory record so enrichment proceeds. M5 — cache key includes the meta.modelsInfoHeaders fingerprint. Switching an `x-tenant` header (or any user-specified meta header) busts the cache. Provider-level headers are intentionally NOT keyed in — a rotating OAuth2 bearer would otherwise thrash the cache on every refresh. Test coverage grew 39 → 52 unit tests; integration still 4. New cases cover header propagation + override, error-log on cache failure, parse error vs empty catalog, best-effort cache write, TTL refresh on 304, and header-keyed cache partitioning. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… time At the end of the config hook, stamp the cached access token onto config.provider[id].options.headers.Authorization for every managed provider. This makes the token visible to subsequent config-time consumers — most notably @vymalo/opencode-models-info, which can now fetch from an OAuth2-protected `meta.modelsInfoUrl` by inheriting options.headers (no coupling between the two plugins). Safety: - Never overwrites a user-set Authorization header (case-insensitive check covers both `Authorization` and `authorization`). - Only propagates when the cached token is still valid (30s skew before declared expiry). Avoids handing a known-stale token to a sibling plugin's config-time fetch. - The chat.headers hook continues to overwrite per-request with a freshly-ensured token, so any drift between config-time and chat-time only ever affects other config-time consumers — never the actual inference call. Logs `oauth2_bearer_propagated_to_provider_headers` (debug) on propagation and `oauth2_bearer_propagation_skipped_user_set` when a user-supplied header wins. Adds two regression tests: - propagates the cached bearer into provider.options.headers - does not overwrite a user-set Authorization header Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0532895dd1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const hasUserAuth = Object.keys(headers).some((k) => k.toLowerCase() === "authorization"); | ||
| if (hasUserAuth) { | ||
| logger.debug("oauth2_bearer_propagation_skipped_user_set", { providerId }); | ||
| return; |
There was a problem hiding this comment.
When the same config object is processed again after the runtime token has been refreshed (for example by chat.headers or the sync scheduler), this guard treats the Authorization header that this hook inserted on the previous run as user-supplied and returns before writing the new cached token. Downstream config-time consumers such as @vymalo/opencode-models-info then keep inheriting the stale bearer and can fail with 401/403 until the config object is rebuilt; track whether the header was plugin-owned or overwrite the previously propagated value when the cached token changes.
Useful? React with 👍 / 👎.
| "files": [ | ||
| "dist" | ||
| ], |
There was a problem hiding this comment.
In the new models-info package, files only whitelists dist, and this package directory does not contain its own LICENSE; npm's packlist includes package.json/README and whitelisted files, but not the repository-root LICENSE for this workspace package (confirmed with npm pack --dry-run, which listed README.md/package.json/dist only). The publish workflow now verifies dist-tarball/models-info/*.tgz and exits when package/LICENSE is absent, so the first release including this package will fail in “Verify tarball contents” before publishing.
Useful? React with 👍 / 👎.
| if npm view "@vymalo/opencode-oauth2@${version}" version >/dev/null 2>&1; then | ||
| echo "::notice::@vymalo/opencode-oauth2@${version} already on npm — skipping." | ||
| exit 0 |
There was a problem hiding this comment.
This commit changes packages/opencode-oauth2/src/opencode.ts, but packages/opencode-oauth2/package.json remains at 0.3.0; with this newly added registry check, a release where @vymalo/opencode-oauth2@0.3.0 already exists exits before pnpm publish. In that common release path the OAuth bearer propagation needed by the new models-info auth composition is never shipped, even though the models-info README tells users to rely on it, so either bump the oauth2 package version or avoid changing it in a release that will skip that package.
Useful? React with 👍 / 👎.
1 participant
Summary
@vymalo/opencode-models-info— second plugin that enriches existing model entries with full metadata (context length, output limit, USD/M-token cost, modalities,tool_call/reasoning/attachmentflags) by fetching from a provider-supplied OpenRouter-shaped endpoint declared viaoptions.meta.modelsInfoUrl.Hooks.confighook after other plugins have populated providers and headers, so it composes with@vymalo/opencode-oauth2, static API keys, or any other auth scheme without depending on any of them. Upstream values are never overwritten.plans/models-info-plan.mddescribing the design decisions, contract, and field mapping.Test plan
pnpm --filter @vymalo/opencode-models-info typecheck— cleanpnpm --filter @vymalo/opencode-models-info test— 19 tests across mapping / cache / pluginpnpm -r typecheck+pnpm -r test— workspace stays green (120 tests total)pnpm -r build— all packages compile🤖 Generated with Claude Code