Skip to content

feat(providers): add Azure Foundry and Bedrock API-key connections - #6032

Open
garvit-arora wants to merge 15 commits into
apache:mainfrom
garvit-arora:feat/azure-foundry-bedrock
Open

garvit-arora wants to merge 15 commits into
apache:mainfrom
garvit-arora:feat/azure-foundry-bedrock

Conversation

@garvit-arora

@garvit-arora garvit-arora commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Add dedicated API-key-only connections for Azure AI Foundry and Amazon Bedrock. Both entries support the provider's applicable Chat Completions, OpenAI Responses, and Anthropic Messages adapters through Maka's shared provider registry. The Desktop protocol picker, request-URL preview, and endpoint guidance cover these choices; CLI onboarding now derives the same supported choices from the registry.

The Azure entry accepts a Foundry endpoint and API key. The Bedrock entry uses the distinct amazon-bedrock-api-key provider id and a Bedrock API key, leaving amazon-bedrock available for the IAM Identity Center proposal in #3370. Entra ID, AWS SigV4, and IAM Identity Center are outside this slice and tracked in #6072. The Desktop descriptions explicitly identify the API-key scope.

The Runtime Host compatibility epoch moves from the current main value 217 to 218 because older clients reject catalog pages containing the new provider types. A mixed-version Host test confirms an epoch-217 client is refused before it can issue a catalog query. The epoch must be rechecked against main and open PRs immediately before merge.

Refs #6031.

Verification

  • Core, Storage, Runtime, Runtime Host, Eval, and CLI builds passed after rebuilding workspaces updated by current main.
  • Focused core provider/catalog tests: 34 passed.
  • Focused CLI onboarding test file: 40 passed.
  • Mixed-version Host handshake test: passed.
  • Biome checks on the changed files, git diff --check, and the protocol epoch guard passed.
  • The broader Windows handshake-compatibility.test file reports read_eof on four existing forged-peer cases. The new mixed-version regression uses the real Host kernel test and passes.

Live verification still required before merge: I have not run a real Azure Foundry or Bedrock request. The issue requires one plain chat turn and one tool call against each provider, plus Bedrock Anthropic Messages, with results recorded here. Intercepted requests and contract tests do not substitute for that run.

AI use

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

Tool(s) and scope: Cursor and OpenAI Codex assisted with implementation, tests, review follow-ups, UI copy, compatibility work, and this description. The new review-follow-up commits carry Generated-by: Codex trailers.

Checklist

  • Tests cover the changed behavior
  • Focused builds, checks, and tests listed above pass
  • Real Azure Foundry and Bedrock acceptance runs recorded

Does this PR entail a change in behavior?

  • Yes — users can create API-key connections for Azure Foundry and Bedrock and choose the applicable protocol.
  • No

@garvit-arora

Copy link
Copy Markdown
Contributor Author

@liugddx Could you review PR #6032 and advise whether the API-key-only catalog slice is useful? I requested a formal reviewer, but GitHub denied RequestReviewsByLogin for my account. I also requested self-assignment on #6031; GitHub denied ReplaceActorsForAssignable. Could you assign #6031 to garvit-arora if the scope looks appropriate?

@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.

Automated review notice: This comment was posted by an automated review agent. It is not an independent human review and does not replace one.

Reviewed range: merge-base c39a7d2 (main) to head df86624, 1 commit, 2 files, +38/-0. This is a first review, so there are no prior findings. No protocol files are touched, so there is no epoch impact.

Verdict: one P2 (PR-caused CI failure) and two P3 notes.

P2: CI test fails on this PR's new entries. provider-catalog-contract.test.ts ("exposes an endpoint source that passes the production baseUrl gate") requires every CATALOG_PROVIDER_TYPES entry to have a non-empty baseUrl, a baseUrlTemplate, or category: 'custom'. Both new entries have baseUrl: '', no template, and category: 'overseas'. CI fails with azure-foundry has no baseUrl, no baseUrlTemplate, and is not a custom connection — it cannot source an endpoint (job 113825199496). This is the only failing test in that run. There are two ways to fix it, and the maintainers should pick one:

  • Give each entry a baseUrlTemplate plus the input that fills it, the way cloudflare-workers-ai does with its account id. For example, Bedrock's region would go into https://bedrock-runtime.${region}.amazonaws.com/openai/v1. This is a larger change, because the add form and submission code special-case Cloudflare by name today.
  • Or deliberately widen the invariant, and the CLI filter below, to admit a user-supplied endpoint for runtimeAdapter.requireBaseUrl providers.

P3: CLI onboarding silently omits both providers. packages/cli/src/onboarding-catalog.ts only offers providers with a baseUrl or category === 'custom', so neither entry appears in the CLI wizard. That filter's comment says only derived-endpoint providers are meant to be excluded. Whichever fix you choose for the P2 should cover this too.

P3: no tests. Nothing pins the two entries: auth kind, requireBaseUrl, discovery mode, and display copy in all three locales. The PR body says tests weren't run locally.

Credential handling and conventions (checked, no issue):

  • Both entries use authKind: 'api_key' and the existing openai-compatible adapter. The desktop add form already asks for a base URL when defaults.baseUrl is empty (provider-add-form.tsx:154, provider-add-submission.ts:148).
  • Submission takes the existing custom_endpoint legacy path (provider-add-submission.ts:66), the same one custom uses. The runtime enforces requireBaseUrl (model-factory.ts:181).
  • The PR adds no new code that stores, logs, or forwards keys, so secrets go through the same storage as every other API-key provider.
  • catalogOrder 42/43 don't collide with other entries. The brand mark falls back to GenericProviderMark. Display copy covers en, zh-CN, and zh-TW, like its neighbours.

Question (not a finding): Azure's OpenAI v1 /models may list catalog base models rather than the user's deployment names, and Bedrock's OpenAI-compatible endpoint may not serve /models at all. I couldn't verify either against a live account. If protocol discovery returns nothing or the wrong names, the empty fallbackModels leaves the user typing the model id by hand. That works, but it may be worth saying so in the description copy.

Mergeability/CI: GitHub reports MERGEABLE, but the merge state is BLOCKED (no approving review). test fails (PR-caused, above); label passes.

},
'azure-foundry': {
label: 'Azure AI Foundry',
baseUrl: '',

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: baseUrl: '' with no baseUrlTemplate and a non-custom category fails the catalog contract test (azure-foundry has no baseUrl, no baseUrlTemplate, and is not a custom connection). Because of the same rule, packages/cli/src/onboarding-catalog.ts also drops this provider from CLI onboarding. The same applies to amazon-bedrock below.

@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.

Automated review notice: This comment was posted by an automated review agent. It is not an independent human review and does not replace one.

Incremental review: df86624 to 02f255b. This adds three commits (75c212a, f230577, 02f255b). The merge-base is unchanged (c39a7d2) and there was no rebase. The net delta touches two files (+17/-2):

  • azure-foundry and amazon-bedrock change from category: 'overseas' to category: 'custom'.
  • A contract test pins both entries.

Prior findings

  • P2 (CI test failed on the endpoint-source invariant): fixed. The PR took the "user-supplied endpoint" route. Both entries keep baseUrl: '' and requireBaseUrl: true and are now category: 'custom', so they pass the provider-catalog-contract endpoint-source check. I checked the side effects of the category change:
    • The only non-test readers of category === 'custom' are the contract test and the CLI onboarding filter.
    • The desktop settings page no longer groups providers by category. provider-endpoint-presentation.ts only special-cases local.
    • Moving these entries out of overseas therefore has no UI side effect.
  • P3 (CLI onboarding omitted both providers): fixed by the same change. listApiKeyOnboardableProviders() (packages/cli/src/onboarding-catalog.ts:41) now lists them with requiresBaseUrl: true, so the wizard collects the endpoint before the API key.
  • P3 (no tests): mostly addressed. The new test pins authKind, the empty baseUrl, openai-compatible with requireBaseUrl, and category. Display copy and discovery mode are not pinned. That is fine for a catalog slice.

New findings in this range: none.

The earlier open question still stands, and it is not a finding. Azure's /models may list base models rather than deployment names, and Bedrock may not serve /models. In either case the user types the model id by hand. The provider description copy could mention this.

No protocol or epoch files are touched.

Mergeability/CI: test passes on 02f255b. GitHub reports the PR as MERGEABLE, and the merge state is BLOCKED only because there is no approving review. No P0-P2 issues remain.

@liugddx liugddx left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for scoping this carefully and asking first. To answer your two questions directly: as written, this slice isn't useful yet, and Bedrock should coordinate with #3370 rather than claim its id.

P1, blocking: both entries are custom minus its protocol adapters, so they can reach fewer models than the existing Custom connection. The custom definition (provider-registry.ts, the entry with category: 'custom') carries protocolAdapters for openai-responses and anthropic-messages. The two new entries keep only the openai-compatible runtime adapter. On top of that, decodeDefaultApiProtocol (runtime-policy/connection-catalog-codec.ts:399-407) rejects a default protocol for anything but custom, and the add form only offers the protocol picker for Custom (provider-add-form.tsx:134). In practice:

  • Claude on Bedrock (Anthropic Messages) can't be reached.
  • Responses-only models on Bedrock and on Azure v1 can't be reached either.
  • Custom reaches all of them today.

Either carry the same protocolAdapters and lift the Custom-only gates for these two types, or drop the entries and document how to configure Custom for Foundry and Bedrock.

P2: the copy and the endpoint help.

  • The copy doesn't say which endpoint shape works. Foundry and Azure need …/openai/v1, not the project or /models endpoints the portal often shows first; Bedrock needs bedrock-runtime.{region}.amazonaws.com/openai/v1.
  • It also doesn't say that deployment or model ids may have to be typed by hand, since /models lists base models, not your deployments.
  • The request-URL preview in provider-endpoint-field.tsx only runs for custom, so these entries lose the one check Custom gives users.

P3: amazon-bedrock is the id #3370 used, with an aws_sso credential kind. That PR (keyuchen21's IAM Identity Center work) was closed by the stale bot on 2026-10-07 for inactivity, not rejected. Landing the same id with api_key would force a storage migration if SSO comes back. Please use a distinct id for an API-key slice, or align the id and credential kind with keyuchen21 first. Reviving #3370 itself is up to its author after a rebase.

P3: the new test only asserts field values. If the entries stay, add a contract case per protocol.

If you take the "full protocol adapters + accurate copy" route, I'm happy to review again.

@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.

Automated review notice: This comment was posted by an automated review agent. It is not an independent human review and does not replace one.

Incremental review: 02f255be to bac2f468. The author added four commits (579da6ba0, b1b9121c0, 3d7ca4bdc, bac2f4689). There was no rebase: the merge-base is still c39a7d24, and range-diff shows the first four commits unchanged. The net delta touches 9 files (+132/-15) and widens the PR beyond a catalog slice:

  • amazon-bedrock is renamed to amazon-bedrock-api-key. No references to the old id remain.
  • Both azure-foundry and amazon-bedrock-api-key gain protocolAdapters for openai-responses (openai adapter, encrypted-content replay) and anthropic-messages (anthropic, auth: 'api-key', normalizeBaseUrl: true).
  • decodeDefaultApiProtocol now lets these two providers store a defaultApiProtocol. openai-chat is allowed through the openai-compatible runtime adapter, and the other protocols only when a matching protocolAdapters entry exists. Every other non-custom provider still rejects one.
  • The desktop add form shows the protocol selector for both providers, the endpoint field shows a request-URL preview and per-provider help text (localized in zh-CN, zh-TW and en), and the tests are extended.

No protocol or epoch files are touched. The host decoder in runtime-host/src/protocol/connection-effects.ts reuses the core codec, and since both provider types are new in this PR, no older Host or client can see them, so there is no compatibility impact.

Prior findings

  • P3, the test doesn't pin display text or model discovery: still open, and now less important. The new contract tests pin the adapters per protocol and the codec acceptance. Display copy and modelDiscovery are still unpinned, which is fine.

Wiring checked: declaredModelApiProtocol (model override, then discovered model, then connection default) feeds resolveModelRuntime, which falls back to defaults.protocolAdapters[apiProtocol]. Model discovery (model-fetcher.ts:208) also selects the list adapter from protocolAdapters[defaultApiProtocol]. So the stored protocol does reach the wire, and does not just get saved and ignored. Per-model apiProtocol overrides are not provider-gated in the codec, so the capability editor's protocol picker, which now appears for these connections because customDefaultApiProtocol is set, saves correctly.

New findings (P3 only):

  • P3: the Azure endpoint help ignores the selected protocol (provider-endpoint-field.tsx:41, copy at settings-provider-copy.ts:276/492/707). Bedrock gets separate help for OpenAI and Anthropic, but Azure always says to use the endpoint "ending in /openai/v1", even when Anthropic Messages is selected. With that base, anthropicV1BaseUrl produces .../openai/v1/messages, which is not the Foundry Anthropic route (.../anthropic/v1/messages). Branch the Azure help on apiProtocol the same way Bedrock does.
  • P3 (unverified, needs confirmation): Anthropic auth mode for Bedrock API keys (provider-registry.ts:1563). auth: 'api-key' sends x-api-key. Bedrock API keys are documented as bearer tokens (Authorization: Bearer ...), and the registry already supports auth: 'bearer'. I could not check this against a live endpoint. Please confirm that the Bedrock /anthropic endpoint accepts x-api-key, or switch to bearer. The same question applies, less sharply, to Azure Foundry.
  • P3: the Bedrock display copy was only updated in English (provider-display-copy.ts:268-270). en now says "Amazon Bedrock (API key)" and mentions Responses and Anthropic Messages. zh-CN and zh-TW keep the old name and the "OpenAI-compatible API" description.

Mergeability/CI: GitHub reports MERGEABLE, with merge state BLOCKED on required review. git merge-tree against current main is clean.

CI test fails on bac2f468, in Desktop e2e only. Two Side Chat specs failed:

  • session-workbar.spec.ts:251 (fork not cleaned up within 10 s)
  • side-chat-followups.spec.ts:80 ("Expected one Side Chat fork, found 0")

I judge these not PR-caused, for three reasons:

  • The PR touches only provider settings, catalog and codec code, and the e2e seed's custom connection decodes exactly as before.
  • In the second spec, the side-chat turn was already running (the Stop button was visible) before the session list came back empty. That points to a listing race, not to model or provider resolution.
  • session-workbar.spec.ts:251 failed the same way on main at 0af3d5ad (run 37869380885), while current main CI is green.

This is the first run on this PR that got as far as e2e. The earlier failures (codec and contract unit tests) are fixed on this head. Please re-run the job to confirm before merging.

Verdict: no P0-P2 defects. The protocol selection is wired end to end. Three P3s remain (Azure help text, Bedrock Anthropic auth mode to confirm, en-only display copy), along with an e2e failure that looks like a flake and needs a re-run.

if (!supportsPreview) return props.children(undefined);
const description = url ? `${copy.requestUrlLabel} ${url}` : undefined;
const endpointHelp = props.providerType === 'azure-foundry'
? copy.azureFoundryEndpointHelp

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.

P3: Azure help ignores apiProtocol. With Anthropic Messages selected, a /openai/v1 base normalizes to .../openai/v1/messages, not Foundry's .../anthropic/v1/messages. Consider branching on the protocol the same way the Bedrock help does.

apiProtocol: 'openai-responses',
responses: { adapter: 'openai', reasoningReplay: 'encrypted-content' },
},
'anthropic-messages': { kind: 'anthropic', auth: 'api-key', normalizeBaseUrl: true },

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.

P3 (unverified): auth: 'api-key' sends x-api-key, but Bedrock API keys are documented as bearer tokens. Please confirm that the Bedrock /anthropic endpoint accepts x-api-key, or use auth: 'bearer'.

'amazon-bedrock-api-key': {
'zh-CN': { name: 'Amazon Bedrock', description: '通过 Bedrock 的 OpenAI 兼容 API、区域终结点和 Bedrock API 密钥连接模型。', badge: 'API' },
'zh-TW': { name: 'Amazon Bedrock', description: '透過 Bedrock 的 OpenAI 相容 API、區域端點與 Bedrock API 金鑰連線模型。', badge: 'API' },
en: { name: 'Amazon Bedrock (API key)', description: 'Use a Bedrock API key with its regional OpenAI-compatible endpoint and select Responses or Anthropic Messages for models that require them.', badge: 'API' },

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.

P3: only the en entry was updated (name "(API key)", plus the mention of Responses and Anthropic Messages). zh-CN and zh-TW above still use the old name and the OpenAI-only description.

@liugddx liugddx left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Approving exact head bac2f4689. Everything from my last review is resolved:

  • P1, protocols. Both entries now carry the same openai-responses and anthropic-messages adapters as custom. decodeDefaultApiProtocol admits a default protocol for these two types only when the registry actually has an adapter for it, and the add form offers the picker for them. Claude on Bedrock and Responses-only models are reachable now.
  • P2, copy and preview. The endpoint help names the right shapes: …/openai/v1 for Foundry, and bedrock-runtime.{region}.amazonaws.com/openai/v1 or /anthropic for Bedrock. AWS documents the latter as accepting a Bedrock API key. The help also says to type deployment or model ids by hand. The request-URL preview now covers both types.
  • P3, the id. amazon-bedrock-api-key no longer collides with #3370's amazon-bedrock/aws_sso, so an SSO revival needs no migration.
  • Tests. The new contract tests pin the Responses adapters, and the codec tests pin which protocols each type accepts.

Non-blocking: "which provider types can choose a protocol" is now spelled out three times, in provider-add-form.tsx, connection-catalog-codec.ts and provider-endpoint-field.tsx. A single helper derived from the registry (a type whose definition has protocolAdapters) would keep a fourth type from updating only some of them.

On CI: the three runs on this head failed in three unrelated places: Side Chat fork e2e, then ShellRunProcessManager supervisor/PTY tests. None of them touches the provider catalog. Because packages/core changed, every dependent workspace suite runs, which makes those intermittent failures more likely to surface. It needs one green run before merge.

Thanks @garvit-arora for turning this around so quickly.

@garvit-arora

Copy link
Copy Markdown
Contributor Author

Thanks for the approval and for the thoughtful review. I built a somewhat similar product in the past, which is why Maka’s problem space felt familiar and caught my interest. I’ve really enjoyed contributing to Maka as a daily side project.\n\nI’ve pushed a follow-up that centralizes protocol-picker and default-protocol support in the provider registry, branches Azure endpoint guidance for Anthropic Messages, and updates the Bedrock API-key auth/localized copy. CI is running on the new head.

@liugddx liugddx left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Re-reviewed exact head 0dc2f71c6. My approval above was for bac2f4689. This commit takes the shared-helper suggestion well: providerSupportsApiProtocolSelection and providerSupportsDefaultApiProtocol now derive from the registry, and the form, codec and endpoint field all use them. I checked that only custom, azure-foundry and amazon-bedrock-api-key satisfy the predicate today, so no other provider gains a protocol picker. The per-protocol help key and its test are good too.

P2, blocking: Bedrock's Anthropic route changed from auth: 'api-key' to auth: 'bearer' (provider-registry.ts, the amazon-bedrock-api-key anthropic-messages adapter). That sends Authorization: Bearer <key>. Every API-key example AWS documents for this route sends the key as x-api-key. That includes bedrock-runtime …/anthropic/v1/messages with -H "x-api-key: $AWS_BEARER_TOKEN_BEDROCK", its streaming variant, and the bedrock-mantle examples; the Anthropic SDK examples pass the key or token as api_key (https://docs.aws.amazon.com/bedrock/latest/userguide/inference-messages-api.html). Unless you've verified Bearer against a real Bedrock API key on this route, please revert to 'api-key', so the shipped default matches the documented header. If Bearer was deliberate, please say what it was tested against, and add a contract test pinning the header either way.

I'm marking this as changes requested only so my earlier approval doesn't carry over to this head. Once the header is settled, I'll approve again.

@garvit-arora

Copy link
Copy Markdown
Contributor Author

Thanks for catching this. I had not verified Bearer against a real Bedrock API key, so I reverted the Bedrock Anthropic adapter to \�uth: 'api-key', which sends the documented \x-api-key\ header. The provider catalog contract now pins that adapter setting.\n\nFocused verification passed: @maka/core\ build plus the provider catalog and runtime-policy codec tests. CI is running on commit \�a6cc699c.

@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.

Automated review notice: This comment was posted by an automated review agent. It is not an independent human review and does not replace one.

Incremental review from bac2f468 to ba6cc699. The author added two commits, 0dc2f71c6 and ba6cc699c. There was no rebase: the merge-base is still c39a7d24. The delta touches 10 files (+105/-26):

  • Two new core helpers, providerSupportsApiProtocolSelection and providerSupportsDefaultApiProtocol, are added in provider-registry.ts:1735-1758 and re-exported from llm-connections.ts. They replace three hardcoded provider lists, in decodeDefaultApiProtocol, provider-add-form.tsx and provider-endpoint-field.tsx.
  • Azure endpoint help now depends on the selected protocol.
  • The zh-CN and zh-TW Bedrock display copy is updated.
  • A comment documents the Bedrock and Azure x-api-key choice.
  • Tests are added.

No protocol or epoch files are touched. The codec's acceptance set is unchanged (see below), so there is no compatibility impact.

Prior findings

  • P3, Azure help ignores protocol: fixed. The new providerEndpointHelpKey (provider-endpoint-field.tsx:54-74) returns azureFoundryAnthropicEndpointHelp for anthropic-messages. That help suggests https://<resource-name>.services.ai.azure.com/anthropic, which normalizeBaseUrl turns into the Foundry /anthropic/v1/messages route. All three locales are updated, and a unit test pins all four help keys.
  • P3, Bedrock Anthropic x-api-key (unverified): resolved, no change needed. The AWS Bedrock user guide ("Inference using Anthropic Messages API") documents curl .../bedrock-runtime.{region}.amazonaws.com/anthropic/v1/messages -H "x-api-key: $AWS_BEARER_TOKEN_BEDROCK" for Bedrock API keys, and the same for bedrock-mantle. So auth: 'api-key' (provider-registry.ts:1563) is correct. The commit title says "use documented ... header", but the commit only adds a test comment; the behavior was already right.
  • P3, Bedrock name and description English-only: fixed. zh-CN and zh-TW now use "Amazon Bedrock(API 密钥/金鑰)" and mention Responses and Anthropic Messages.
  • Prior desktop e2e Side Chat failure: resolved as a flake. Desktop e2e passes on this head.

Refactor equivalence:

  • providerSupportsApiProtocolSelection requires category: 'custom', the openai-compatible adapter with requireBaseUrl: true, and non-empty protocolAdapters (with custom short-circuited to true). In the registry, the only category: 'custom' entries are azure-foundry (:1545), amazon-bedrock-api-key (:1569) and custom (:1643), so the derived set equals the old hardcoded list.
  • providerSupportsDefaultApiProtocol keeps the old per-protocol rule: openai-chat requires the openai-compatible adapter, other protocols require a protocolAdapters entry, and custom accepts any isModelApiProtocol.
  • providerDefaultsOf('custom') is defined, so the new !defaults early return cannot reject custom connections.
  • Future registry entries that match the shape will gain the picker automatically. That is intentional and pinned by the new contract test.

New findings: none.

Mergeability/CI: GitHub reports MERGEABLE, with merge state BLOCKED on required review. git merge-tree against current main is clean.

CI test fails on ba6cc699, only in Storybook smoke: product-settings-pages--usage-long-tail and --usage-narrow fail with "expected 168.03125 to be less than or equal to 168" (settings-pages.stories.tsx:2835). This is a known main-side flake, not PR-caused. The PR touches no stories or usage code. All other steps pass, including Desktop e2e. A re-run should clear it.

Verdict: no P0-P2 defects. All prior P3s are fixed or resolved, and the only CI failure is the known main-side Storybook flake.

@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.

Automated review notice: This comment was posted by an automated review agent. It is not an independent human review and does not replace one.

At 5ac8b20adfb3e8da2a558fdb35aefb37cadb972b, the earlier endpoint/catalog issues, Responses/Anthropic adapter gates, provider-ID collision, localized setup instructions, and Bedrock Anthropic authentication regression are resolved. The current Anthropic adapter uses x-api-key, consistent with the Bedrock Messages documentation and Azure Foundry Claude API-key documentation. The previous changes-requested review still needs its author's reconsideration; an older approval does not cover this head.

[P2] Allocate a new compatibility epoch for the new provider values — packages/core/src/provider-registry.ts:1528.

These registry entries change accepted Runtime Host wire values even though no file under the protocol directory changes. Connection catalog responses carry providerType; decodeCatalogConnectionHeader calls the core decodeProviderType, which rejects providers absent from that client's registry. With both versions advertising epoch 212, an older client can connect to the updated Host but rejects a catalog page containing either new provider as invalid_frame: connection provider type is not registered. I reproduced this using the actual base core decoder and the unchanged production protocol decoder: Custom pages pass both versions; Azure Foundry and Bedrock pages pass the new decoder and fail the old one.

Move the compatibility epoch and add a regression for this mixed-version case. Main currently uses 212; open #5394/#5969 use 213, and #5548/#6051 use 214. Across 188 currently open PR heads, 215 is the next unused value. Recheck allocation immediately before merging.

[P3] CLI onboarding still offers only the default Chat Completions path. packages/cli/src/onboarding-catalog.ts:48 expands protocol choices only for custom. Actual catalog execution returns three Custom choices, but one choice without defaultApiProtocol for each new provider. Desktop exposes all three protocols. Reuse the registry-based capability helper for CLI choices, or document this limitation; Custom remains a workaround.

[P3] Update the PR description. It still describes a Chat-Completions-only slice and contains no statement of whether generative tools made a substantive contribution, as required by CONTRIBUTING.md. Please state the actual protocol scope and the applicable AI-use answer; missing disclosure is not evidence that AI was used.

Validation: 309 focused Node tests, 32 Chromium provider/settings renders, forced builds of nine dependency projects, Desktop main build, and all four Desktop typechecks passed. Six intercepted requests through the actual runtime model factory verified endpoint, model and authentication construction for both providers across all three protocols; no real cloud requests or credentials were used. Live cloud authentication and response interoperability remain unverified. The one-pixel Usage assertion also passed the existing LongTail/Narrow browser scenarios.

This head's CI test succeeded (run 38041097328). GitHub reports MERGEABLE/BLOCKED; automatic merging with current main bacb1caed is textually clean. Neither green CI nor a clean textual merge resolves the compatibility failure above.

signupUrl: 'https://dash.cloudflare.com/profile/api-tokens',
catalogOrder: 33,
},
'azure-foundry': {

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] Advance the compatibility epoch when adding these wire-visible provider values

An epoch-212 client built before these entries can still connect to this epoch-212 Host, but connection.catalog.query responses now contain providerType values absent from the old registry. The unchanged decodeCatalogConnectionHeader delegates to core decodeProviderType and rejects the entire page with invalid_frame ("connection provider type is not registered"). The actual base decoder accepts a Custom control page but rejects both new provider pages, while the head decoder accepts all three. This is a wire compatibility change even without edits under src/protocol. Allocate a new epoch and cover the mixed-version case. Current main is 212; #5394/#5969 use 213 and #5548/#6051 use 214. 215 is currently free across the 188 open heads checked, but recheck before merging.

@liugddx liugddx left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Re-reviewed exact head 5ac8b20ad, superseding my changes-requested on 0dc2f71c6. The Bedrock Anthropic adapter is back on the documented x-api-key, the helpers derive the protocol picker from the registry, the Azure help follows the selected protocol, and all three locales are updated. Thanks for turning those around.

Still blocking:

P2: wire compatibility (agreeing with Astro-Han). connection-effects.ts:314 decodes each catalog connection's providerType with the core decodeProviderType (connection-catalog-codec.ts:823-827), which rejects any type missing from the client's own registry. An older client at the same epoch therefore connects to an updated Host and then rejects every catalog page that contains azure-foundry or amazon-bedrock-api-key. Please bump RUNTIME_HOST_COMPATIBILITY_EPOCH and add a mixed-version regression test. Note that main has since moved to 215 (#6051), so 215 is no longer free: pick the next unused value against open PRs immediately before merge.

Issue #6031's own acceptance criterion. #6031 says to "verify provider setup and tool-call behavior through Maka's shared provider contracts before treating either entry as complete". So far the evidence is intercepted requests and contract tests; nobody has run a real Azure Foundry or Bedrock request. Before this lands, please run each provider once against a real endpoint and record it in the PR: one plain chat turn and one tool call, on at least the default protocol. Do the same for Anthropic Messages on Bedrock, since its auth header is the part most likely to differ.

PR description. It still describes a Chat-Completions-only slice, says tests weren't run, and has no AI-use answer (required by CONTRIBUTING). Please update it to the actual scope: three protocols, the new provider id, and the epoch.

Non-blocking: CLI onboarding (packages/cli/src/onboarding-catalog.ts:48) still expands protocol choices only for custom. Reusing providerSupportsApiProtocolSelection there would make the CLI match Desktop.

The product-scope question on #6031 (whether to ship these API-key entries ahead of #860's Azure/Bedrock design) is still open with the maintainers. I'll answer it there.

@liugddx

liugddx commented Oct 11, 2026

Copy link
Copy Markdown
Member

Scope is now decided on #6031: ship the API-key slice with the three protocols, and track cloud-native auth separately in #6072. My changes-requested stands only for the merge conditions listed there:

The non-blocking CLI onboarding note (reuse providerSupportsApiProtocolSelection in onboarding-catalog.ts) still applies.

@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.

Automated review notice: This comment was posted by an automated review agent. It is not an independent human review and does not replace one.

At df579c381da5140c54f21876157d3266d945e355, the previously reported compatibility P2 and both P3 notes are resolved. I found no new P0–P2 issue in this increment.

  • The Host epoch is now 218, above current main's 217. A real Host/transport regression rejects an epoch-217 client before it can issue catalog commands. My decoder comparison still reproduces why the separation is needed: main's registry rejects the two new provider types, while this head accepts them. A separate forged-epoch-217 Host probe confirms the current client rejects the reverse mismatch before sending a domain command.
  • CLI onboarding now derives all three supported protocol choices from the registry, preserving the selected protocol in the create target.
  • The three-language Desktop descriptions explicitly identify API-key-only support, and the PR description now states its actual scope, remaining live-verification work, and generative-tool use.

The original eleven implementation commits remain unchanged; I separated the main merge from the three authored follow-ups. Fresh installation and the normal full root build passed. The complete provider/catalog/CLI files passed 72 tests, the targeted real Host regression passed, and the complete client-handshake file passed six tests. Formatting, whitespace, and the epoch guard passed. Six intercepted production SDK requests again confirmed the protocol endpoints and authentication headers; no real cloud request was made.

This is not yet ready to merge. The maintainer decision on #6031 accepts the API-key scope but requires real Azure Foundry and Bedrock chat/tool calls, including Bedrock Anthropic Messages. The current description explicitly says those runs remain unperformed. Contract tests and intercepted requests do not satisfy that condition; please record the required live results before requesting final approval.

The exact-head CI is successful and the main merge is clean. A fresh scan of all 179 open PR heads found no other epoch 218; #5969 remains at 216. Recheck the live epoch allocation immediately before merging. I did not dismiss the existing requested-changes review or treat this automated comment as approval.

This branch has not been deployed

No deployments
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.

3 participants