Skip to content

fix(runtime): fall back Responses relay discovery to /v1 - #3797

Open
Sandu1213 wants to merge 1 commit into
apache:mainfrom
Sandu1213:fix/responses-relay-api-base-url
Open

fix(runtime): fall back Responses relay discovery to /v1#3797
Sandu1213 wants to merge 1 commit into
apache:mainfrom
Sandu1213:fix/responses-relay-api-base-url

Conversation

@Sandu1213

@Sandu1213 Sandu1213 commented Aug 25, 2026

Copy link
Copy Markdown

Summary

A custom OpenAI Responses relay configured with its host root can pass the connection probe at <root>/responses while publishing its model catalog at /v1/models. Discovery previously stopped after <root>/models returned 404, so the connection looked verified but its catalog never refreshed.

This revision preserves the configured API base as the primary contract and adds a narrow discovery fallback:

  1. Send and probe keep using the configured Responses endpoint.
  2. Discovery first requests <configured-base>/models.
  3. Only when a pathless root returns 404, discovery retries the same-origin /v1/models route once.
  4. Authorization, network, and invalid-response failures remain authoritative; no POST request is replayed.
  5. Endpoint-form overrides such as …/v1/responses are reduced to their API base before deriving …/v1/models.
configured send / probe discovery
http://relay.example:3000 /responses /models, then /v1/models only after 404
https://relay.example/v1 /v1/responses /v1/models
https://relay.example/v1/responses /v1/responses /v1/models
https://relay.example/relay/v1 /relay/v1/responses /relay/v1/models

Fixes #3320

Compatibility and review response

This supersedes the earlier root → /v1 normalization and addresses the P2 compatibility review: persisted root-mounted relays continue to use /responses and /models exactly as before. The fallback is provider-scoped to openai-responses-compatible, same-origin, GET-only, 404-only, and attempted once. Built-in providers such as DeepSeek retain their unversioned root behavior.

Regression coverage locks:

  • a root-mounted relay succeeds without any /v1 request;
  • a relay whose root /models route is absent falls back once to /v1/models while probe/send stay at /responses;
  • a plain openai-compatible relay surfaces its root 404 without retrying /v1/models;
  • a path-mounted relay (…/gateway) surfaces its 404 without appending /v1 to its own prefix;
  • an endpoint-form base (…/v1/responses) discovers through /v1/models and nothing else;
  • a 401 is not hidden behind the fallback;
  • built-in-provider send behavior remains unchanged.

The three negative discovery cases matter more after the rebase: #4411 dispatches discovery on runtimeAdapter.kind and merged openai with openai-compatible into a single branch, so the exact providerType check and the pathless-root check are now the only things holding the fallback's scope. Each of those cases fails if its guard is removed.

Verification

Rebased onto b1369c808 (main). The two conflicts were in packages/runtime/src/model-fetcher.ts (PROVIDER_DEFAULTSPROVIDER_REGISTRY, and switch (definition.protocol)switch (definition.runtimeAdapter.kind) with openai / openai-compatible merged) and in provider-conformance.test.ts (both sides appended tests; both are kept).

Passed:

  • npm --workspace @maka/core run build, npm --workspace @maka/storage run build, npm --workspace @maka/mcp run build, npm --workspace @maka/runtime run build
  • node --test dist/__tests__/provider-conformance.test.js dist/__tests__/provider-contract-matrix.test.js dist/__tests__/responses-wire-contract.test.js in packages/runtime — 178 passed, 0 failed
  • npx biome check on the three touched files — clean
  • npm run check:asf-headers — clean

npm --workspace @maka/runtime run test:dist on this head: 3,249 tests, 3,232 passed, 13 skipped, 4 failed. The same 4 fail on unmodified b1369c808 (3,241 tests, 3,224 passed, 13 skipped, 4 failed): openai-responses-plaintext-reasoning, one Alibaba Responses reasoning-replay expectation, task-ledger-tools.test.js, and tool-catalog-derive.test.js, which still cannot import the unexported @maka/core/tool-catalog subpath. None touch this PR's paths.

AI use

Select exactly one:

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope: Claude Opus 5 performed the original investigation and implementation, then rebased onto the latest main, resolved the two conflicts, and added the three negative discovery regressions. OpenAI Codex (GPT-5.6) traced the compatibility path, replaced the root-rewriting behavior with the 404-only discovery fallback, added the compatibility/error-boundary regressions, and reviewed the conflict resolution. The author owns the submission.

Checklist

  • Tests cover the change and fail on the missing fallback
  • The affected package build, targeted suites, lint, and format checks pass

Does this PR entail a change in behavior?

  • Yes — described under Summary and Compatibility above
  • No

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I reviewed this head and found no blocking issues.

The relay base normalization now consistently heals bare roots to /v1 and strips trailing /responses at the endpoint, aligning probe, discovery, and send. Scoped only to openai-responses-compatible to avoid breaking built-in providers that serve both paths. Tests lock the five normalization cases.

No P0-P2. Checks on afb7b5d have no hosted report yet — not green.

简体中文该头未发现阻断。

@M4n5ter
M4n5ter force-pushed the fix/responses-relay-api-base-url branch 3 times, most recently from 5487df2 to fb95dc1 Compare August 26, 2026 09:56
@github-actions github-actions Bot added the effort/M Under 500 readable lines label Aug 27, 2026

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for working on the probe/discovery/send drift. I need to supersede my earlier review on afb7b5d: it missed the existing root-mounted custom-relay configuration described inline.

The single-base direction is sound, but choosing /v1 for every pathless custom relay is a product-contract change rather than a universally safe normalization. I’m leaving this as a P2 COMMENT—not a request for changes—but I would not approve until the intended compatibility behavior is explicit.

Exact head reviewed: fb95dc1a0f5136b5b1501e9123da01972fa31651. Hosted checks are green.

中文对照

需要纠正我在旧 head 上的结论:统一入口的方向正确,但把所有无路径 Relay 自动改成 /v1 会改变已有配置含义。这里定为 P2 Comment,不发 Request Changes,但建议先明确兼容契约再批准。

AI-assisted review: Codex traced the compatibility path and drafted this feedback; the maintainer verified the conclusion and severity.

Comment thread packages/runtime/src/provider-urls.ts Outdated
return baseUrl;
}
const basePath = stripTrailing(url.pathname).replace(/\/responses$/i, '');
url.pathname = basePath === '' ? '/v1' : basePath;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[P2] Preserve or explicitly migrate the existing root-mounted relay contract

Thanks for centralizing the URL derivation. This line does more than normalize equivalent spellings, though: a pathless custom Responses relay previously meant /responses and /models at the configured root, while it now always means /v1/responses and /v1/models.

Root-mounted custom relays are a valid existing configuration, so this silently breaks them on the normal send/discovery path. Please either preserve root semantics and use a deterministic /v1 fallback, or make /v1 the explicit product contract with migration/UI/documentation and a regression showing the old shape is intentionally unsupported.

Because this changes the meaning of persisted user configuration, a Discussion establishing the contract would also be preferable to treating it as an implementation-only correction.

中文对照

这里不只是统一 URL 写法,而是把已有的“根路径提供 /responses/models”配置重新解释成 /v1。这会直接破坏合法的现有自定义 Relay。建议保留根路径语义并确定性 fallback 到 /v1,或者明确迁移产品契约;这种行为变化最好先通过 Discussion 达成共识。

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.

Addressed in b8ff53a69 after rebasing onto the latest main.

The bare-root → /v1 rewrite is removed. A root-mounted relay remains authoritative for send, probe, and the first discovery request (/responses and /models). Only a 404 from that pathless root model route triggers one same-origin GET fallback to /v1/models; 401/network/invalid-response failures do not fall back, and POST requests are never replayed. Endpoint-form overrides still reduce to their API base for discovery.

Regression coverage now proves the existing root contract, the 404-only fallback, the authorization boundary, and unchanged DeepSeek root behavior. The PR title/body and verification notes have also been updated.

@Sandu1213
Sandu1213 force-pushed the fix/responses-relay-api-base-url branch from fb95dc1 to b8ff53a Compare August 30, 2026 13:38
@Sandu1213 Sandu1213 changed the title fix(runtime): resolve a Responses relay from one API base fix(runtime): fall back Responses relay discovery to /v1 Aug 30, 2026
@Sandu1213
Sandu1213 force-pushed the fix/responses-relay-api-base-url branch from b8ff53a to 5708460 Compare September 4, 2026 13:49
A root-mounted custom OpenAI Responses relay remains authoritative for send,
probe, and discovery. When only its root /models route returns 404, retry
discovery once at /v1/models; do not retry authorization failures or POST
requests.

Endpoint-form overrides still reduce to their API base before discovery, so
/v1/responses discovers through /v1/models without changing persisted root
semantics.

The retry stays scoped to a pathless openai-responses-compatible base: a plain
OpenAI-compatible relay and a path-mounted Responses relay surface the 404
without a second request. The discovery regressions lock that boundary now
that both OpenAI-shaped adapters share one discovery wire.

Refs apache#3320

Generated-by: Claude Opus 5
Generated-by: OpenAI Codex (GPT-5.6)
@Sandu1213
Sandu1213 force-pushed the fix/responses-relay-api-base-url branch from 5708460 to 6e22e67 Compare September 4, 2026 13:57
@Sandu1213

Copy link
Copy Markdown
Author

Rebased onto b1369c808 and force-pushed as 6e22e6773; the branch is mergeable again. No behavior was renegotiated — the diff is still the same 3 files, and the fallback contract is exactly the one described above.

Two conflicts, both from #4411 (Host owns the model catalog):

  • packages/runtime/src/model-fetcher.tsPROVIDER_DEFAULTSPROVIDER_REGISTRY, and switch (definition.protocol)switch (definition.runtimeAdapter.kind) with case 'openai' and case 'openai-compatible' merged into one branch. Resolved by keeping main's registry/dispatch and this PR's endpoint-form reduction plus the 404-only retry inside the merged branch.
  • packages/runtime/src/__tests__/provider-conformance.test.ts — purely additive on both sides; main's stream_options usage loop and this PR's relay cases are both kept.

That merge makes the guard load-bearing in a way it was not before: openai-compatible now enters the same discovery branch, so the exact providerType check and the pathless-root check are the only things scoping the retry. Three negative discovery regressions were added for that:

  • a plain openai-compatible relay surfaces its root 404 with ['/models'] and no /v1/models;
  • a path-mounted relay (…/gateway) surfaces its 404 with ['/gateway/models'] and never appends /v1;
  • an endpoint-form base (…/v1/responses) discovers through ['/v1/models'] only.

Each fails when its guard is removed — verified by dropping the providerType condition, the pathless-root check, and the openAiResponsesBaseUrl reduction in turn.

Verification on this head: @maka/core / @maka/storage / @maka/mcp / @maka/runtime builds clean; the three targeted suites are 178/178; biome check and check:asf-headers clean. The full @maka/runtime test:dist has 4 failures that also fail on unmodified b1369c808, all outside this PR's paths.

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

Labels

effort/M Under 500 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Custom OpenAI Responses relay does not append /v1 when fetching models

2 participants