Skip to content

feat: filter discovered models with include/exclude globs - #4

Open
fsx8 wants to merge 3 commits into
yourcasualdev:mainfrom
fsx8:feat/model-filter
Open

feat: filter discovered models with include/exclude globs#4
fsx8 wants to merge 3 commits into
yourcasualdev:mainfrom
fsx8:feat/model-filter

Conversation

@fsx8

@fsx8fsx8 commented Aug 6, 2026

Copy link
Copy Markdown

Summary

Adds includeModels and excludeModels plugin options so that not every model exposed by CLIProxyAPI's /v1/models endpoint has to be exposed to OpenCode. Useful when CLIProxyAPI fronts many providers/models but you only want a specific subset available in OpenCode's /models picker.

Filtering runs after discovery and before the provider is merged into the OpenCode config, so provider behavior (protocol routing, capability hints, user model overrides) is unchanged.

How it works

  • Both options accept arrays of glob patterns matched against the model IDs reported by CLIProxyAPI:
    • * matches any run of characters
    • ? matches a single character
    • all other characters (including .) are matched literally
  • includeModels keeps only matching models; excludeModels drops matching models.
  • excludeModels takes precedence over includeModels when both match.
  • If a filter would remove every discovered model, the plugin throws at startup so the misconfiguration is not silently ignored.
{
"plugin": [
[
"opencode-cliproxyapi",
{
"baseURL": "http://your-server:8317",
"apiKey": "your-key",
"includeModels": ["claude-*", "gpt-5.*"],
"excludeModels": ["*-image"]
}
]
]
}

Implementation

  • src/catalog.ts: add globToRegExp (tiny, dependency-free glob -> RegExp) and filterModels(models, { include, exclude }), plus an exported ModelFilter type.
  • src/index.ts: parse the two options in readOptions, apply filterModels to the discovered catalog, throw when the result is empty, and surface the filter in the discovery log line.
  • README.md: document the new options (table + a "Filtering discovered models" section) with examples.
  • CHANGELOG.md: entry under [Unreleased].

Notes

  • Per CONTRIBUTING.md this is a user-visible behavior change; I went straight to a PR with tests + docs for review rather than opening an issue first, but happy to split it out into an issue for discussion if you prefer.
  • Filtering is case-sensitive to match the exact IDs CLIProxyAPI reports.

Checklist

  • bun run check passes (typecheck + 32 tests + build; was 14 before)
  • Tests added for globToRegExp, filterModels, and the plugin
    (include, exclude-wins, and empty-result error path)
  • No new dependencies
  • No credentials committed

Add includeModels and excludeModels plugin options so only a subset of
the models discovered from CLIProxyAPI's /v1/models endpoint is exposed
to OpenCode. Entries are glob patterns (* and ?) matched against model
IDs; excludeModels takes precedence over includeModels. Filtering to
zero models throws at startup so misconfiguration is not silently
ignored.

@chatgpt-codex-connectorchatgpt-codex-connectorBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit:be84aaad9a

ℹ️ 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".

Comment threadsrc/index.ts
apiKey,
protocol: options.protocol ?? "chat",
catalog,
catalog: filteredCatalog,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Keep excluded existing models out of the provider

Passing filteredCatalog here does not fully enforce the new filters because addProvider later merges existing?.models back into the provider via mergeModels(discovered, existing?.models). In any config that already has a customized or stale entry for a model that the filter excludes, such as includeModels: ["claude-*"] with an existing gpt-5.6-terra override, that model is still exposed in OpenCode even though the log reports it was filtered out. Please apply the same filter when preserving existing model entries, or only merge overrides for models that survived filtering.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch — fixed in fcb4512.

The root cause was mergeModels re-merging every entry from the existing provider model config, so an override for a filtered-out model (e.g. includeModels: ["claude-*"] with a pre-existing gpt-5.6-terra override) still leaked through.

Fix:addProvider/mergeModels now take an optional allowedModelIDs set, populated with the surviving discovered IDs only when a filter is active. The merge loop skips any existing entry not in that set, so overrides are preserved solely for models that survived filtering. With no filter configured, allowedModelIDs is undefined and behavior is unchanged (existing models still merge as before).

Added a regression test (filtering drops existing overrides for excluded models but keeps included ones) covering exactly the scenario you described: a gpt-5.6-terra override is dropped under includeModels: ["claude-*"], while a claude-sonnet-4-6 override is still applied. bun run check passes (33 tests).

mergeModels previously re-merged every entry from existing provider
model config, so a user override for a model the filter excludes (e.g.
includeModels: ["claude-*"] with an existing gpt-5.6-terra override)
was still exposed. Pass an allowlist of surviving model IDs (only when
a filter is active) so overrides are preserved only for models that
survive filtering. No behavior change when no filter is configured.
@yourcasualdev

Copy link
Copy Markdown
Owner

Thanks for the PR — this is well put together. Focused scope, dependency-free glob implementation, unit + plugin tests, and both README and CHANGELOG updated. I checked out the branch and bun run check is green here too (typecheck + 33 tests + build, no new deps).

One real issue with the follow-up fix in fcb4512, plus a doc gap and some nits.

The override fix drops models the filter never targeted

allowedModelIDs is built from the surviving discovered models:

allowedModelIDs: filtering
? newSet(filteredCatalog.map((model)=>model.id))
: undefined,

and then mergeModels skips any existing entry not in that set:

for(const[modelID,model]ofObject.entries(existing??{})){
if(allowedModelIDs&&!allowedModelIDs.has(modelID))continue

So a model the user declared by hand in their own OpenCode config — one CLIProxyAPI doesn't report — is deleted the moment any filter is set. Two cases I confirmed against the branch:

  • excludeModels: ["*-image"] plus a hand-added my-hand-added-model → dropped, even though it doesn't match the exclude glob. An exclude-only filter shouldn't remove something that doesn't match.
  • includeModels: ["claude-*"] plus a hand-added claude-experimental-preview → dropped, even though it does match the include glob.

The second case misses the rule the CHANGELOG states ("overrides are only preserved for models that survive filtering") — that model survives the filter, it just wasn't discovered. Before this PR hand-added models were always preserved, so this is a silent regression for anyone combining a filter with a manual model entry.

The fix is to filter existing IDs by the filter rather than by the discovered set — pass the ModelFilter down instead of a Set<string>:

if(modelFilter&&filterModels([{id: modelID}],modelFilter).length===0)continue

That still covers what Codex originally reported (a gpt-5.6-terra override under includeModels: ["claude-*"] fails the include and gets dropped) without eating entries the filter never named. Could you add a test for the exclude-only + hand-added case? Nothing covers it right now.

README should mention what happens to existing model entries

The CHANGELOG says overrides are only preserved for surviving models, but the README describes the options purely as filters on "discovered model IDs" and doesn't say what happens to entries in the user's own provider.cliproxyapi.models. Whichever semantics we settle on, that's the surprising part and it belongs in the README.

Nits

  • escapeRegExp in src/catalog.ts: * and ? are in the character class but unreachable, since the loop handles them earlier. - is emitted as \- outside a character class, which is only legal via Annex B — fine today, but it would break if the regex ever gained the u flag.
  • filtering in src/index.ts is derived from the raw option arrays while filterModels independently recomputes emptiness with its own nonEmpty. They agree today only because stringArrayOption already drops blank entries — deriving filtering from what filterModels actually did would remove the coupling.
  • Pathological globs (*a*a*a*…) can backtrack, but patterns are user-authored config and model IDs are short, so I don't think it's worth handling.

No need for an issue first — the PR is a fine place to have had this discussion. Happy to merge once the allowedModelIDs change and the README line are in.

Gating mergeModels on the surviving discovered IDs dropped hand-added
models the filter never targeted: an exclude-only filter removed
non-matching manual entries, and an include filter dropped manual
entries matching the include glob that CLIProxyAPI does not report.
Pass the ModelFilter down instead and test each existing entry against
it, so overrides survive exactly when they match the filter. Also
derive the filtering flag via hasModelFilter to share emptiness logic,
and make glob escaping unicode-mode-safe.
@fsx8

fsx8 commented Aug 21, 2026

Copy link
Copy Markdown
Author

Thanks for the thorough review — you're right on all counts. I verified both cases against the branch: gating mergeModels on the surviving discovered IDs meant any filter (even exclude-only) silently deleted hand-added entries the filter never named. That was a regression I introduced in fcb4512, and your second case even contradicted the CHANGELOG wording. Fixed in a86c41c.

Main fix:addProvider/mergeModels now take the ModelFilter itself instead of the ID set, and each existing entry is tested against the filter exactly as you sketched:

if(modelFilter&&filterModels([{id: modelID}],modelFilter).length===0)continue

So the original Codex report stays fixed (a gpt-5.6-terra override under includeModels: ["claude-*"] fails the include and is dropped), while your two cases now behave correctly: my-hand-added-model survives excludeModels: ["*-image"], and a hand-added claude-experimental-preview survives includeModels: ["claude-*"] with its customizations applied.

Tests: added the exclude-only + hand-added case you asked for, plus the include + hand-added-matching case, and a hand-added entry that does match an exclude pattern (dropped).

README: added a paragraph to "Filtering discovered models" describing what happens to provider.cliproxyapi.models entries, including hand-added models CLIProxyAPI does not report.

Nits (addressed):

  • escapeRegExpChar now only escapes actual regex syntax characters (^$\.*+?()[]{}|/), no Annex B-only \-, and sources are verified u-flag-safe by a test.
  • Added hasModelFilter in catalog.ts sharing the same nonEmpty logic as filterModels; index.ts derives filtering from it instead of duplicating emptiness rules.
  • Agreed on backtracking — left as-is for the reasons you gave.

bun run check passes (39 tests). Happy to adjust further.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@fsx8@yourcasualdev