fix(opencode): advertise per-model image capabilities in the exported config - #4293
fix(opencode): advertise per-model image capabilities in the exported config#4293colthreepv wants to merge 1 commit into
Conversation
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks 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 |
|
✅ Deterministic PR hygiene checks passed. |
⏳ DRAFT
What to do
Review readiness checklist
2/4 boxes ticked. This PR stays in draft until every box above is ticked. |
4e543d4 to
ab26dde
Compare
… config
The OpenCode export emitted only `name` and `limit` per model, so opencode computed
`capabilities.attachment` and `capabilities.input.image` as false for every opencodex model:
the provider is absent from models.dev, and opencode's loader falls back to a hardcoded false
for an entry that says nothing. Attachments were then refused client-side in the TUI before any
request reached the proxy — including native OpenAI slugs that /api/models reports as
["text","image"], and text-only models the vision sidecar covers.
Carry the catalog row's `inputModalities` through `OpencodeCatalogModel` and
`opencodeCatalogFromProxyRows`, then serialize them as opencode's own per-model fields
(`attachment`, `modalities`) as declared by opencode's published model schema. Both provider
generations get them, so the two spellings of one model list cannot disagree; the V2 model
schema expresses capabilities as `capabilities.{tools,input,output}` (which opencode fills by
migrating this same `modalities` field) and its loader decodes with
`onExcessProperty: "ignore"`. A row that declares nothing keeps the previous entry shape, which
opencode already treats as text-only.
Values outside opencode's enum (text|audio|image|video|pdf) are dropped rather than written
through, the way `audio` had to be for Pi and Gajae; a row left with nothing acceptable keeps
its entry without capability keys instead of being retyped as text.
`exportModelsFromProxyRows` no longer re-joins modalities by `namespaced` — the catalog entry
carries them now, from the same visibility-filtered row as the model itself, so a disabled
duplicate cannot donate them.
Closes lidge-jun#4286
리뷰 · 우선순위 70 / 80이 PR은 OpenCode로 내보내는 모델 목록에 이미지(첨부) 가능 여부를 제대로 적어 주는 수정입니다. 지금 고치는 길은 짧고 분명합니다. 방금 라인 - (브랜치 메인테이너의 판단이 필요한 지점
너의 추천 이 댓글은 grok-bot이 작성했습니다 |
ab26dde to
9ad2057
Compare
Summary
opencode reads per-model image support from the model entry itself and cannot consult models.dev for
opencodex— the provider is not listed there — so its loader falls back to a hardcodedfalsefor an entry that declares neitherattachmentnormodalities. The OpenCode export wrote onlynameandlimit, which cataloged every routed model as text-only and made the TUI refuse an image paste client-side. No request reached the proxy, so the vision sidecar never ran either.The affected set was not limited to genuinely blind models: native OpenAI slugs whose
/api/modelsrow reports["text","image"]were blocked the same way, as were text-only models the sidecar covers.The row's declared
inputModalitiesnow travels throughOpencodeCatalogModel/opencodeCatalogFromProxyRowsand is serialized as opencode's own per-model fields on both provider generations:Three decisions carry the shape of the change:
text|audio|image|video|pdf(opencode.ai/config.json), wider than the internaltext|image|audiovocabulary, so nothing the proxy can produce today is filtered. The guard exists becauseaudioalready took a whole config file down for Gajae, whose loader rejects the file over one out-of-enum value. Where Pi and Gajae drop a row they cannot represent, opencode keeps it without capability keys rather than receiving text it cannot read.attachment/modalitiesare declared by opencode's V1 model schema. Its V2 model schema expresses the same information ascapabilities.{tools,input,output}, which V2 reaches by migrating the V1 block. Both exported blocks therefore carry these fields; native V2 consumption of the extra keys is not established.This is the same class of defect already fixed for the Codex app in #344/#349 and already handled by the Hermes/Pi/Gajae exporters (#3146); the OpenCode serializer was missed.
One pre-existing defect is fixed along the way because the change moves the data:
exportModelsFromProxyRowsre-joined modalities from the raw/api/modelsrows keyed bynamespacedwith the first row winning, so a disabled or hidden duplicate could donate its modality list to the visible entry — the donation the availability and effort-ladder rules already refuse. Modalities now come from the same visibility-filtered catalog row as the model itself.Out of scope, stated rather than silently widened. Custom rows take a different path through
listManagementModelRows, which rebuilds the row fromconfig.customModelsand copies only the stored modalities, discarding the sidecar-addedimagethatgatherRoutedModelsappended atsrc/codex/catalog/provider-fetch.ts:2649. A custom model with storedinputModalities: ["text"]whose provider also lists it innoVisionModelstherefore still reaches opencode as text-only. That projection affects every client export built from this management-row projection, not only this one, and belongs in its own change; the guide documents the row-level rule and names this case rather than claiming sidecar coverage unconditionally.Upstream verification
The emitted field names, the five-value enum, and the
attachmentsemantics come from opencode's publishedconfig.jsonand from the loader that reads them (packages/opencode/src/provider/provider.ts:1523), so the V1 contract is both documented and loadable. The V2 path was read, not run:packages/core/src/config.tsandpackages/core/src/v1/config/migrate.tsshow V2 reaching these capabilities by migrating the V1 block. Native V2 consumption of the extra keys is not established here, so a reviewer who needs a V2 guarantee should confirm it against the target version.Verification
9ad205758, based ondevd9849942a. It has since been rebased locally ontodevcb7f96cbc, the tip that landed theomoclient, which also changessrc/clients/config-export.ts, itscontracts.tsandtests/config/client-config-export.test.ts. The rebase applied with no conflicts and altered none of this change's own bytes (same 10 files, +275/-27), but the checks below ran on that local rebase rather than on9ad205758; the difference is the base, and the extra base commits are not exercised by them.bun run typecheck— clean (native TypeScript 7.0.2, 1423 files).bun run privacy:scan— passed.bun test tests/config/client-config-export.test.ts tests/providers/opencode-cli.test.ts tests/clients/client-export-modality-enum.test.ts tests/server/management-client-config-route.test.ts tests/test-layout-tooling.test.ts tests/test-layout.test.ts— 195 pass, 0 fail. No test file was added, so no layout manifest entry was needed.9ad205758is green: 24 checks succeeded and 2 were skipped by design (windows <shard>/6,macos control), includingtest 1/4to4/4,gates,storage policy,api usage,docker smoke, and thekeyringandnpm-globaljobs on Ubuntu, Windows and macOS.devworktree at29d632ff2rather than assumed environmental —pnpm generated shims > verifies POSIX shims point at the active packageand> accepts a pnpm group alias when it resolves to the active package,test runner isolation > keeps the .NET known-folder lookup resolvable inside the sandbox, and the twobearer admission is not reused as a Cursor upstream credentialcases, which time out at 5s. Two of the seven counted failures aretests/routing/routing-policy-pool-quota.test.tsandtests/routing/routing-profile.test.ts, whose workers crashed with exit code 3 under the 4-way parallel run; both pass in isolation on the rebased head (35 pass, 0 fail), and routing is untouched here. The tally's single1 erroris unnamed in the captured recap and is left unattributed rather than called pre-existing.code: 124, so the figures above come from the complete pre-rebase run rather than a re-derivation on the rebased tree.New coverage, all behavioral:
tests/providers/opencode-cli.test.ts— the launcher path, proxy rows to catalog toOPENCODE_CONFIG_CONTENT: a declared["text","image"]row reaches both blocks, a text-only row is advertised text-only rather than omitted, an undeclared row carries no capability keys, a disabled row stays filtered.tests/clients/client-export-modality-enum.test.ts— the enum boundary, in the file that already guards Pi, Gajae and Hermes: a live["text","image","audio"]row is carried whole,["audio"]stays audio-only, an out-of-enum value is dropped with the entry retained, and no model in a whole catalog carries a value opencode rejects.tests/config/client-config-export.test.ts— Fast rows inherit the capabilities of the row they duplicate (a Fast user selects exactly that selector), each generation owns itsmodalitiesmap, and a disabled duplicate cannot donate its modality list.tests/server/management-client-config-route.test.ts— the/api/client-configgolden for the route the dashboard download uses, updated to the new entry shape.Checklist
Docs:
docs-site/src/content/docs/guides/opencode.mdgains an "Images and attachments" section stating where the capability fields come from, that an undeclared row stays plain, that a discovered model picks up the sidecar image, and that a custom row is written from the modalities stored on it. The seven translated copies of that page (fr, ja, ko, ru, tr, zh-cn, zh-tw) are untouched and none of them makes a claim about attachments that this section contradicts.No security-relevant surface is touched. The change consumes catalog metadata the proxy already reports at
GET /api/modelsand filters it to opencode's allowed values; admission, credentials, and headers are unchanged.Closes #4286
Review readiness checklist
This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:
All CI tests are green on my local testing.
I pushed my PR to the latest dev commit.
I resolved all correct Codex and CodeRabbit findings.
My PR is ready for review.
done with ds41flash help, reviewed by astra (limitations noted above).