fix(sse): stop unhydrated compatible connections routing to the real OpenAI/Anthropic API (#13452) by diegosouzapw · Pull Request #13798 · diegosouzapw/OmniRoute

diegosouzapw · 2026-09-15T20:35:38Z

Fixes the reported Bug 2 (the issue title): buildUrl() silently defaulting an
openai-compatible-*/anthropic-compatible-* connection to the real OpenAI/Anthropic API when
its providerSpecificData.baseUrl was absent.

Not covered here (out of scope for a surgical, TDD-provable fix — see the plan file's own
scoping notes):

Bug 1 (CLI keys add credential-attach discoverability gap) — a documentation/UX gap, not a
data-loss or security bug; the working path (POST /api/providers) already exists.
Bug 3 (nodes add --base-url CLI collision) — already fixed in fix(cli): support --base-url alongside --endpoint in nodes subcommands (#11999) #12033, predates this issue.
The open-sse/services/provider.ts buildProviderUrl()/getProviderConfig() literal-fallback
pattern flagged by the plan as "not yet read" — it backs only the translator/validation debug
routes (src/app/api/translator/{send,translate}/route.ts, src/lib/providers/validation.ts),
not the primary chat pipeline, and those call sites already resolve baseUrl explicitly before
calling it. Worth a narrower follow-up if it turns out to matter in practice.

Root cause

BaseExecutor.buildUrl() (open-sse/executors/base.ts) and DefaultExecutor.buildUrl()
(open-sse/executors/default.ts, the class actually instantiated for a bare
openai-compatible-*/anthropic-compatible-* provider id) both read
credentials.providerSpecificData.baseUrl and, when it was missing, silently fell back to the
literal https://api.openai.com/v1 / https://api.anthropic.com/v1 instead of erroring or
re-resolving the node. providerSpecificData.baseUrl is only ever stamped onto a connection at
write time by two routes (POST /api/providers's hydration branch, and the node-update backfill
loop in PUT /api/provider-nodes/{id}) — any connection created outside those two paths (a direct
DB insert, a row predating the hydration logic, or a race with node update/deletion) has no
providerSpecificData.baseUrl and reproduced this bug: the connection's own stored "API key" is
shipped as a Bearer/x-api-key token to a public third party instead of the operator's intended
local/self-hosted endpoint.

Fix

Fail loudly instead of silently defaulting. requireCompatibleBaseUrl()
(open-sse/config/providerRegistry.ts) is the new shared guard both executors' buildUrl()
call for the openai-compatible-*/anthropic-compatible-* branches — it throws
provider node "<id>" has no baseUrl — node missing or connection not hydrated instead of
defaulting to a real third-party API.
Read-path self-heal. hydrateConnectionProviderSpecificData()
(src/sse/services/compatibleNodeBaseUrl.ts, wired into materializeConnection() in
src/sse/services/auth.ts) re-joins provider_nodes (via the existing 5s-TTL
getCachedProviderNodes() cache — no extra DB read on the hot path) and stamps
baseUrl/prefix/apiType/nodeName/chatPath/modelsPath/customHeaders onto the
credentials before they ever reach the executor, so any connection missing the write-time copy
self-heals instead of needing the exact original write path. This is also where the (unrelated,
pre-existing) Proxy Pool by-id hydration used to live — moved here verbatim (file-size-baseline.json
freezes auth.ts's line count, so new logic goes in a new module rather than growing it).

Regression test

tests/unit/issue-13452-node-baseurl-ignored.test.ts (new file) — 4 cases: DefaultExecutor and
BaseExecutor openai-compatible throw when unhydrated, DefaultExecutor anthropic-compatible
throws when unhydrated, and a control case confirming a hydrated connection still routes
correctly.

RED on unfixed code (executors reverted to HEAD via git show HEAD:<path>, no stash used):

✖ issue #13452: DefaultExecutor openai-compatible buildUrl must not silently fall back to the real OpenAI API when providerSpecificData.baseUrl is absent
  AssertionError [ERR_ASSERTION]: Missing expected exception: ...
✖ issue #13452: BaseExecutor openai-compatible buildUrl must not silently fall back to the real OpenAI API when providerSpecificData.baseUrl is absent
✖ issue #13452: DefaultExecutor anthropic-compatible buildUrl must not silently fall back to the real Anthropic API when providerSpecificData.baseUrl is absent
ℹ tests 4
ℹ pass 1
ℹ fail 3

GREEN on fixed code:

✔ issue #13452: DefaultExecutor openai-compatible buildUrl must not silently fall back to the real OpenAI API when providerSpecificData.baseUrl is absent
✔ issue #13452: BaseExecutor openai-compatible buildUrl must not silently fall back to the real OpenAI API when providerSpecificData.baseUrl is absent
✔ issue #13452: DefaultExecutor anthropic-compatible buildUrl must not silently fall back to the real Anthropic API when providerSpecificData.baseUrl is absent
✔ issue #13452 (control): providing providerSpecificData.baseUrl routes correctly (confirms the fallback, not buildUrl() itself, was the defect)
ℹ tests 4
ℹ pass 4
ℹ fail 0

Existing tests aligned

tests/unit/executor-default-base.test.ts — two anthropic-compatible-cc-test fixtures
(DefaultExecutor.execute tests) relied on the old silent-default behavior by omitting
providerSpecificData.baseUrl. Aligned to the corrected contract by adding a hydrated
baseUrl: "https://cc.test/v1" to each fixture (a real connection produced by the
POST /api/providers hydration branch always carries one) — no assertion was weakened, the tests
verify header/reasoning-effort behavior unrelated to the baseUrl value itself.

Gates run

npx eslint --suppressions-location config/quality/eslint-suppressions.json <changed files> — clean, exit 0.
npm run typecheck:core — clean.
npm run check:open-sse-typecheck — OK, 0 pre-existing errors within frozen baseline.
node scripts/check/check-file-size.mjs — no ✗ on touched files (one pre-existing, unrelated
violation on open-sse/utils/stream.ts, confirmed via git diff --stat to be untouched by this
PR).
node scripts/check/check-test-discovery.mjs — OK, new test file discovered.
Focused regression suite: tests/unit/issue-13452-node-baseurl-ignored.test.ts (4/4),
tests/unit/{cc-compatible-provider,executor-default-base,ollama-local-provider,newapi-gateway-providers,xiaomi-providers-registry,custom-endpoint-paths}.test.ts (128/128),
tests/unit/{10085-compatible-generic-vs-uuid-credential,provider-node-select-4421,provider-service}.test.ts (21/21).

…OpenAI/Anthropic API (#13452) Root cause: BaseExecutor/DefaultExecutor.buildUrl() silently defaulted an openai-compatible-*/anthropic-compatible-* connection to the real OpenAI/Anthropic API when providerSpecificData.baseUrl was absent, shipping the connection's own stored credential to a public third party. baseUrl is only ever stamped at write time by two routes; a connection created outside those (direct DB insert, pre-hydration row, node-update race) reproduced this bug. Fix: buildUrl() now throws instead of defaulting (requireCompatibleBaseUrl in providerRegistry.ts), and the credential-selection read path (auth.ts -> compatibleNodeBaseUrl.ts) self-heals by re-joining provider_nodes via the existing cache before a request ever reaches the executor. Regression test: tests/unit/issue-13452-node-baseurl-ignored.test.ts

…red (base-red fix #13747)

…soning-effort test (diegosouzapw#13866) - Run agent skills generator to sync skills/cli-mcp/SKILL.md with mcp enable/disable subcommands added in diegosouzapw#13770 - Hydrate providerSpecificData.baseUrl in SimpleExecutor for reasoning-effort-clamp-and-retry.test.ts to satisfy diegosouzapw#13798 requireCompatibleBaseUrl guard Signed-off-by: Minxi Hou <houminxi@gmail.com>

… guard

* fix(quality): clear the release/v3.8.51 base-reds 19 failing unit tests plus the API Route Typecheck and mutation-test-coverage gates, all reproduced on the clean tip before touching anything. Ten of the failures share one cause. #13452/#13798 made `*-compatible-*` buildUrl() refuse a connection with no baseUrl instead of quietly defaulting to the real OpenAI/Anthropic API — which would ship the operator's stored key to a public third party. The guard is right; three fixtures still built those connections unhydrated, and one of them put baseUrl at the top level of credentials, where the chat path never reads it. The rest: - modelDiscovery.ts missed the VertexModelMetadataProvenance cast that its read-path twin in db/models/synced.ts already had — both written by #12471. - A provider-test regexp carried raw 0x00/0x1f bytes, which makes git, GitHub and ripgrep treat the file as binary. Same character class, written with escapes instead of the bytes themselves. - #13399 (Agnes AI China) adds "agnes-cn" + "agnescn": the only two provider prefixes since the count was last set (412 -> 414). Everything else added in that range is model ids. - The free-tier budget card SVG was stale (443 -> 452 models); regenerated by its own script. - Three new tests were missing from stryker.conf.json tap.testFiles, so the mutants they kill did not count. Three guards asserted syntax rather than the invariant they protect, and broke when the source legitimately changed. Each was re-expressed and then verified by mutating the source back: - #2331 required modelEffort to head the rawEffort chain; #13556 deliberately put the server-selected force rule first. The real invariant is relative — modelEffort outranks the defaults a client injects — and it still trips when explicitReasoning is moved ahead of it. - The OAuth loopback guard matched the isLocalhost arm literally; #9944 added `&& !opts?.manualLoopback`. It now matches the arm whatever guards it, and still fails when the hint stops being built. - The i18n scanner flagged dynamically-built keys — t("effort." + mode) reaches it as a literal prefix, never a string. It now accepts a prefix that resolves to a namespace holding messages, and still fails when the namespace is gone. tests/unit/sse-auth.test.ts (#12080) expected a bare null where #13879 now returns the key-policy diagnostic — the same sentinel shape the terminal-state path has used since #12441. The assertion was rewritten to the constraint #12080 actually protects: nothing usable comes back and neither connection leaks. The contract risk that remains — those sentinels are truthy, and executeWebSearch treats any truthy value as a credential — is filed as #13945 rather than widened into this PR. Refs #13866 * fix(quality): clear the second wave of release/v3.8.51 base-reds The tip moved 13 commits while the first pass was running and brought its own reds. All reproduced locally on the merged tree first. vitest 4.1.11 -> 5.0.0 in the #13661 development-group bump is a major, and vitest 5 moved `vite` from a dependency to a peerDependency. This repo only ever declared `vite` under `overrides`, which pins a version but installs nothing, so `npm ci` stopped providing it and the Vitest job died at startup with ERR_MODULE_NOT_FOUND. Declared as the devDependency it actually is — the same ^8.0.16 the override already pinned, and what @vitejs/plugin-react asks for as a peer — and regenerated the lockfile: 684 lines added, none changed. #12909 filtered a mapped array with `toolCall is JsonRecord`, but the element type is the tool-call literal or null, and a predicate's type has to be assignable to the parameter's (TS2677). Narrowed by the element's own type instead; the literal still satisfies JsonRecord at the return. #12906 added `|| result.errorCode === "empty_response"` to the stream-failure condition and Prettier rewrapped it, so the #8928 probe — which located the branch by an exact four-line string — stopped finding it. It now matches on what the branch tests rather than how it is typeset, and still fails when the eviction call is removed. probe-7293 is the visible half of a real conflict, filed as #13948. #7293 merges a mid-array system into index 0; #12908, landed later, demotes it to "user" in place instead. Both target the same constraint and only one can win, and the combination also reorders: the pre-translation hoist moves the turn forward expecting it to stay a system message, then the demotion converts it where it now sits, ahead of the conversation. Choosing between the two strategies is a product call, not a base-red one, so the test was realigned to assert the half that protects the caller — the instruction survives, as a user turn — and pins the current ordering with a pointer to the issue, so the eventual decision shows up as a deliberate test change instead of a silent regression. Refs #13866, #13948 * fix(quality): allowlist vite, rebaseline tip growth, drop a dead import Third pass on the release/v3.8.51 base-reds. Declaring `vite` in the previous commit was correct but incomplete: check-deps is a human review point against typosquatting, so a newly declared package has to be vouched for by name. Recorded in dependency-allowlist.json with why it is needed — the official Vite build tool, already pinned through overrides, and a required peer of both vitest 5 and @vitejs/plugin-react. That also turns check-deps.test.ts green. check-file-size went red on nine files. One is mine: sse-auth.test.ts grew when the #12080 assertion was rewritten. Three of the four assertions I had added were redundant with the strict deepEqual that follows them, so they are gone and the file grows by 4 lines instead of 8; the cap absorbs the rest. The other eight are production and test files this PR does not touch, grown by other work and never rebaselined — which is the whole reason a base-red drain exists. Each is attributed to the commit that grew it: #12906 (chat.ts, chatHelpers.ts, proxyFetch.ts, stream.ts), #12904 + #12910 (chatCore.ts), and batch_api.test.ts from the same wave. Two of them predate the wave entirely and were already over cap on 3d5baf1 — imageGeneration.ts (#13748) and roundRobinCombo.ts (#13776) — so they were base-reds hiding behind a gate that only surfaced them once the tip was merged in. Both are recorded separately from the wave so the history stays honest about when each cap actually moved. Note for whoever reads the gate next: it counts one line more than `wc -l`, since it measures split length rather than newlines. Finally, #13290 replaced rmSync with cleanupTempDataDir in zcode-executor.test.ts but left the import behind, which the frozen-warning ESLint gate rejects. Removed. Refs #13866

…OpenAI/Anthropic API (diegosouzapw#13452) (diegosouzapw#13798) Merged in the 2026-09-16 sweep of the maintainer's own open PRs, at the owner's explicit instruction. No push was made to the PR branch: the merge took the head as the owning session left it (verified OPEN, non-draft and MERGEABLE against the release tip immediately before merging).

* fix(quality): clear the release/v3.8.51 base-reds 19 failing unit tests plus the API Route Typecheck and mutation-test-coverage gates, all reproduced on the clean tip before touching anything. Ten of the failures share one cause. diegosouzapw#13452/diegosouzapw#13798 made `*-compatible-*` buildUrl() refuse a connection with no baseUrl instead of quietly defaulting to the real OpenAI/Anthropic API — which would ship the operator's stored key to a public third party. The guard is right; three fixtures still built those connections unhydrated, and one of them put baseUrl at the top level of credentials, where the chat path never reads it. The rest: - modelDiscovery.ts missed the VertexModelMetadataProvenance cast that its read-path twin in db/models/synced.ts already had — both written by diegosouzapw#12471. - A provider-test regexp carried raw 0x00/0x1f bytes, which makes git, GitHub and ripgrep treat the file as binary. Same character class, written with escapes instead of the bytes themselves. - diegosouzapw#13399 (Agnes AI China) adds "agnes-cn" + "agnescn": the only two provider prefixes since the count was last set (412 -> 414). Everything else added in that range is model ids. - The free-tier budget card SVG was stale (443 -> 452 models); regenerated by its own script. - Three new tests were missing from stryker.conf.json tap.testFiles, so the mutants they kill did not count. Three guards asserted syntax rather than the invariant they protect, and broke when the source legitimately changed. Each was re-expressed and then verified by mutating the source back: - diegosouzapw#2331 required modelEffort to head the rawEffort chain; diegosouzapw#13556 deliberately put the server-selected force rule first. The real invariant is relative — modelEffort outranks the defaults a client injects — and it still trips when explicitReasoning is moved ahead of it. - The OAuth loopback guard matched the isLocalhost arm literally; diegosouzapw#9944 added `&& !opts?.manualLoopback`. It now matches the arm whatever guards it, and still fails when the hint stops being built. - The i18n scanner flagged dynamically-built keys — t("effort." + mode) reaches it as a literal prefix, never a string. It now accepts a prefix that resolves to a namespace holding messages, and still fails when the namespace is gone. tests/unit/sse-auth.test.ts (diegosouzapw#12080) expected a bare null where diegosouzapw#13879 now returns the key-policy diagnostic — the same sentinel shape the terminal-state path has used since diegosouzapw#12441. The assertion was rewritten to the constraint diegosouzapw#12080 actually protects: nothing usable comes back and neither connection leaks. The contract risk that remains — those sentinels are truthy, and executeWebSearch treats any truthy value as a credential — is filed as diegosouzapw#13945 rather than widened into this PR. Refs diegosouzapw#13866 * fix(quality): clear the second wave of release/v3.8.51 base-reds The tip moved 13 commits while the first pass was running and brought its own reds. All reproduced locally on the merged tree first. vitest 4.1.11 -> 5.0.0 in the diegosouzapw#13661 development-group bump is a major, and vitest 5 moved `vite` from a dependency to a peerDependency. This repo only ever declared `vite` under `overrides`, which pins a version but installs nothing, so `npm ci` stopped providing it and the Vitest job died at startup with ERR_MODULE_NOT_FOUND. Declared as the devDependency it actually is — the same ^8.0.16 the override already pinned, and what @vitejs/plugin-react asks for as a peer — and regenerated the lockfile: 684 lines added, none changed. diegosouzapw#12909 filtered a mapped array with `toolCall is JsonRecord`, but the element type is the tool-call literal or null, and a predicate's type has to be assignable to the parameter's (TS2677). Narrowed by the element's own type instead; the literal still satisfies JsonRecord at the return. diegosouzapw#12906 added `|| result.errorCode === "empty_response"` to the stream-failure condition and Prettier rewrapped it, so the diegosouzapw#8928 probe — which located the branch by an exact four-line string — stopped finding it. It now matches on what the branch tests rather than how it is typeset, and still fails when the eviction call is removed. probe-7293 is the visible half of a real conflict, filed as diegosouzapw#13948. diegosouzapw#7293 merges a mid-array system into index 0; diegosouzapw#12908, landed later, demotes it to "user" in place instead. Both target the same constraint and only one can win, and the combination also reorders: the pre-translation hoist moves the turn forward expecting it to stay a system message, then the demotion converts it where it now sits, ahead of the conversation. Choosing between the two strategies is a product call, not a base-red one, so the test was realigned to assert the half that protects the caller — the instruction survives, as a user turn — and pins the current ordering with a pointer to the issue, so the eventual decision shows up as a deliberate test change instead of a silent regression. Refs diegosouzapw#13866, diegosouzapw#13948 * fix(quality): allowlist vite, rebaseline tip growth, drop a dead import Third pass on the release/v3.8.51 base-reds. Declaring `vite` in the previous commit was correct but incomplete: check-deps is a human review point against typosquatting, so a newly declared package has to be vouched for by name. Recorded in dependency-allowlist.json with why it is needed — the official Vite build tool, already pinned through overrides, and a required peer of both vitest 5 and @vitejs/plugin-react. That also turns check-deps.test.ts green. check-file-size went red on nine files. One is mine: sse-auth.test.ts grew when the diegosouzapw#12080 assertion was rewritten. Three of the four assertions I had added were redundant with the strict deepEqual that follows them, so they are gone and the file grows by 4 lines instead of 8; the cap absorbs the rest. The other eight are production and test files this PR does not touch, grown by other work and never rebaselined — which is the whole reason a base-red drain exists. Each is attributed to the commit that grew it: diegosouzapw#12906 (chat.ts, chatHelpers.ts, proxyFetch.ts, stream.ts), diegosouzapw#12904 + diegosouzapw#12910 (chatCore.ts), and batch_api.test.ts from the same wave. Two of them predate the wave entirely and were already over cap on 8a95ffa — imageGeneration.ts (diegosouzapw#13748) and roundRobinCombo.ts (diegosouzapw#13776) — so they were base-reds hiding behind a gate that only surfaced them once the tip was merged in. Both are recorded separately from the wave so the history stays honest about when each cap actually moved. Note for whoever reads the gate next: it counts one line more than `wc -l`, since it measures split length rather than newlines. Finally, diegosouzapw#13290 replaced rmSync with cleanupTempDataDir in zcode-executor.test.ts but left the import behind, which the frozen-warning ESLint gate rejects. Removed. Refs diegosouzapw#13866

maxmad64bis mentioned this pull request Sep 15, 2026

fix(sse): skip already-refused route per request on 429 #13795

Merged

5 tasks done

chore: sync release/v3.8.51 into fix/13452-provider-node-baseurl-igno…

be9f1ca

…red (base-red fix #13747)

diegosouzapw merged commit 2bbe6e5 into release/v3.8.51 Sep 16, 2026
17 of 21 checks passed

HouMinXi mentioned this pull request Sep 16, 2026

fix(ci): heal release/v3.8.51 base-reds for agent-skills sync and reasoning-effort test (#13866) #13880

Closed

patrykkopycinski added a commit to patrykkopycinski/OmniRoute that referenced this pull request Sep 16, 2026

test: hydrate baseUrl in strip-markers fixture after diegosouzapw#13798…

9507621

… guard

patrykkopycinski added a commit to patrykkopycinski/OmniRoute that referenced this pull request Sep 16, 2026

test: hydrate baseUrl in strip-markers fixture after diegosouzapw#13798…

1b59bd0

… guard

diegosouzapw mentioned this pull request Sep 17, 2026

fix(quality): clear the release/v3.8.51 base-reds #13947

Merged

diegosouzapw mentioned this pull request Sep 17, 2026

🔴 Release branch not green: release/v3.8.51 #13866

Closed

Navigation Menu

Uh oh!

fix(sse): stop unhydrated compatible connections routing to the real OpenAI/Anthropic API (#13452) - #13798

fix(sse): stop unhydrated compatible connections routing to the real OpenAI/Anthropic API (#13452)#13798
diegosouzapw merged 2 commits into
release/v3.8.51from
fix/13452-provider-node-baseurl-ignored

diegosouzapw commented Sep 15, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

diegosouzapw commented Sep 15, 2026

Root cause

Fix

Regression test

Existing tests aligned

Gates run

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant