Skip to content

fix(opencode): require an http(s) baseURL in both OpenCode plugins - #13142

Merged
diegosouzapw merged 1 commit into
diegosouzapw:release/v3.8.51from
maxmad64bis:fix/plugin-v2-api-url
Sep 11, 2026
Merged

diegosouzapw merged 1 commit into
diegosouzapw:release/v3.8.51from
maxmad64bis:fix/plugin-v2-api-url

Conversation

@maxmad64bis

@maxmad64bis maxmad64bis commented Sep 9, 2026 •

Copy link
Copy Markdown
Contributor

⚠️ base-red inherited: #12732

Summary

Type your gateway address without http:// and the plugin accepts it, then nothing works.

z.string().url() accepts anything new URL() parses, and new URL() reads localhost:20128 as the scheme localhost: followed by a path. Both OpenCode plugins take it, publish every model with localhost:20128/v1 as its api url, and every call then fails inside the client on an unknown scheme. No request is sent, so the gateway logs show nothing and the error names neither the option nor a model.

Only host:port slips through — or.example.com/v1 is already rejected — which is the local address almost every install starts with.

Both plugins now trim the value and require an http(s) scheme, the same treatment headroomUrl already gets in src/shared/validation/settingsSchemas.ts, where the comment spells out why z.string().url() is not enough on an address that ends up in fetch. Inside the v2 plugin a single isHttpUrl backs the option schema, the publish boundary (legacyApiToInfoApi) and the snapshot filter (isStaleSnapshotModel), which previously accepted a card with a blank or relative url — those three cannot drift apart. The v1 plugin carries its own copy of the predicate, since the two packages ship independently and neither depends on the other.

Related Issues

Validation

  • Change type: provider / routing / UI / i18n / CLI / DB / build-deploy / other
  • Focused tests and category gates from the golden path
  • npm run lint
  • Reconciled with the current active release base; focused checks rerun afterward
  • Production-code changes include a new or updated automated test in this PR

Tests Added Or Updated

  • @omniroute/opencode-plugin/tests/options-schema.test.ts and @omniroute/opencode-plugin-v2/tests/options.test.ts — scheme-less, ftp:// and bare-host addresses rejected with the new message; http(s) accepted, padding trimmed.
  • @omniroute/opencode-plugin-v2/tests/snapshot-stale-entries.test.ts — a snapshot entry with a package but no url is dropped and counted in the warning; blank, relative and non-http urls rejected at the publish boundary.
  • docs/guides/OPENCODE-V2-PLUGIN.md — the baseURL row now states the http(s) requirement.

Coverage Notes

  • Touches src/index.ts in @omniroute/opencode-plugin, and src/options.ts, src/catalog.ts, src/cache.ts, src/shared/models-map.ts in @omniroute/opencode-plugin-v2. Neutering isHttpUrl turns three v2 cases red, one per site; removing either plugin's schema guard turns its own case red.

Reviewer Notes

  • This is a tightening: a configuration accepted today stops being accepted. It never worked — it failed on every call with an error that named nothing. A clear message at startup replaces a catalog of unusable models.
  • The three red checks on this head are inherited, not from this branch:
    • No new ESLint warnings — 🔴 Release branch not green: release/v3.8.51 #12732 states ratchet drift (eslint warnings, cognitive complexity, file size) is expected mid-cycle, rebaselined at release, and not a contributor concern. ESLint is clean on the five files this PR changes.
    • Merge integrity (changelog + generated skills) — 🔴 Release branch not green: release/v3.8.51 #12732 lists check:agent-skills-sync among the base failures.
    • Docs Gates (fast-path) — check:docs-counts reports three stale migration counts, in README.md, AGENTS.md and llm.txt. This PR touches none of them; the only doc it changes is the plugin guide.
  • npm run lint is left unchecked for the same reason: the repo gate is red on this base for three no-explicit-any errors in tests/unit/volcengine-plan-binding-upsert.test.ts, untouched here.
  • Run each package suite with its own npm test: v2 is 222/222; v1 is 368/369, where scaffold.test.ts imports a built dist/ the tree does not carry and fails the same way on an untouched checkout.

@maxmad64bis
maxmad64bis force-pushed the fix/plugin-v2-api-url branch from 0acff67 to f8fa1eb Compare September 9, 2026 23:45
@maxmad64bis maxmad64bis closed this Sep 9, 2026
@maxmad64bis maxmad64bis changed the title fix(plugin-v2): refuse to publish a model whose api block has no url fix(plugin-v2): require an http(s) baseURL and a routable api url Sep 10, 2026
@maxmad64bis maxmad64bis reopened this Sep 10, 2026
@maxmad64bis
maxmad64bis force-pushed the fix/plugin-v2-api-url branch 3 times, most recently from 1cbe16a to bcb0f16 Compare September 10, 2026 00:09
@maxmad64bis maxmad64bis changed the title fix(plugin-v2): require an http(s) baseURL and a routable api url fix(opencode-plugin): require an http(s) baseURL in both plugins Sep 10, 2026
@maxmad64bis
maxmad64bis force-pushed the fix/plugin-v2-api-url branch 2 times, most recently from 6a09ece to 00217a9 Compare September 10, 2026 00:18
@maxmad64bis maxmad64bis changed the title fix(opencode-plugin): require an http(s) baseURL in both plugins fix(opencode): require an http(s) baseURL in both OpenCode plugins Sep 10, 2026
`z.string().url()` accepts anything `new URL()` parses, and `new URL()` reads
`localhost:20128` as the scheme `localhost:` followed by a path. A gateway
address typed without `http://` passed validation in both plugins, every model
was published with `localhost:20128/v1` as its api url, and every call failed in
the client on an unknown scheme — no request on the wire, nothing in the gateway
logs, no model named. Only `host:port` slips through, since `or.example.com/v1`
is already rejected; that is the local-instance form most installs start from.
Both schemas now trim and require an http(s) scheme, the same treatment
`headroomUrl` already gets in `src/shared/validation/settingsSchemas.ts`, where the
comment spells out why `z.string().url()` is not enough on an address that ends
up in `fetch`.

In the v2 plugin the same predicate guards the two places a model card is
judged. `legacyApiToInfoApi` refused an api block without `npm` but returned
`url` unchecked, and `isStaleSnapshotModel` looked at `npm` alone; the host
reads `api.url` in `prepareOptions` and never falls back to the provider's own,
so a card carrying `/v1` or a blank string reaches the AI SDK with nothing to
call. One `isHttpUrl` in the package's url module now backs the option schema,
the publish boundary and the snapshot filter, so the three cannot drift.

Tests: v2 222/222, v1 368/369 (`scaffold.test.ts` needs a built `dist/`, which
the tree does not carry). Neutering `isHttpUrl` turns three v2 cases red, one
per site; removing either plugin's schema guard turns its own case red.
@maxmad64bis
maxmad64bis marked this pull request as draft September 10, 2026 02:17
@maxmad64bis
maxmad64bis marked this pull request as ready for review September 10, 2026 09:17
@diegosouzapw
diegosouzapw merged commit 3156643 into diegosouzapw:release/v3.8.51 Sep 11, 2026
21 of 38 checks passed
Githab-capibara added a commit to Githab-capibara/OmniRoute that referenced this pull request Sep 17, 2026
…iegosouzapw#13142)

Real and nasty precisely because it is silent: `z.string().url()` accepts `localhost:20128` as scheme `localhost:` plus a path, every model gets published with an unusable api url, and the failure happens inside the client so the gateway logs show nothing. Backing the option schema, the publish boundary and the snapshot filter with one `isHttpUrl` in v2 is the right call — those three cannot drift apart. Duplicating the predicate in v1 rather than sharing it is also correct, since the two packages ship independently.

---

Validated in one consolidated worktree cut from `release/v3.8.51`, boarded together with the rest of this batch — zero conflicts between them.

- `typecheck:core` clean; `check:changelog-integrity` OK
- complexity 2799 / baseline 3218 and cognitive-complexity 1265 / baseline 1437 — both under baseline
- 86 focused assertions green across the batch's 10 unit test files, plus 16/16 on the v1 plugin option schema and 16/16 on the v2 option tests
- `check-file-size` rebaselined for this batch's real growth (annotation `_rebaseline_2026_09_11_mergebatch_v3851_maxmad_opencode`, landed on diegosouzapw#13141). `open-sse/utils/stream.ts` was deliberately left frozen: it is already 3115 > 3098 on the pure tip with zero contribution from this batch.

⚠️ base-red inherited: diegosouzapw#12732 — `Docs Gates`, `Merge integrity`, `No new ESLint warnings`, `Unit Tests fast-path` and `Fast Quality Gates` all reproduce on the pure `release/v3.8.51` tip (provider count 356 vs the 358 the modules define, SKILL.md drift, and the `stream.ts` freeze above). None of them touch these diffs.

Thanks @maxmad64bis.
@maxmad64bis
maxmad64bis deleted the fix/plugin-v2-api-url branch September 24, 2026 21:15
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…iegosouzapw#13142)

Real and nasty precisely because it is silent: `z.string().url()` accepts `localhost:20128` as scheme `localhost:` plus a path, every model gets published with an unusable api url, and the failure happens inside the client so the gateway logs show nothing. Backing the option schema, the publish boundary and the snapshot filter with one `isHttpUrl` in v2 is the right call — those three cannot drift apart. Duplicating the predicate in v1 rather than sharing it is also correct, since the two packages ship independently.

---

Validated in one consolidated worktree cut from `release/v3.8.51`, boarded together with the rest of this batch — zero conflicts between them.

- `typecheck:core` clean; `check:changelog-integrity` OK
- complexity 2799 / baseline 3218 and cognitive-complexity 1265 / baseline 1437 — both under baseline
- 86 focused assertions green across the batch's 10 unit test files, plus 16/16 on the v1 plugin option schema and 16/16 on the v2 option tests
- `check-file-size` rebaselined for this batch's real growth (annotation `_rebaseline_2026_09_11_mergebatch_v3851_maxmad_opencode`, landed on diegosouzapw#13141). `open-sse/utils/stream.ts` was deliberately left frozen: it is already 3115 > 3098 on the pure tip with zero contribution from this batch.

⚠️ base-red inherited: diegosouzapw#12732 — `Docs Gates`, `Merge integrity`, `No new ESLint warnings`, `Unit Tests fast-path` and `Fast Quality Gates` all reproduce on the pure `release/v3.8.51` tip (provider count 356 vs the 358 the modules define, SKILL.md drift, and the `stream.ts` freeze above). None of them touch these diffs.

Thanks @maxmad64bis.
Sign up for free to 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