Skip to content

Add Atlas Cloud agent provider - #581

Open
binyangzhu000-sudo wants to merge 3 commits into
EtienneLescot:nextfrom
binyangzhu000-sudo:codex/add-atlas-cloud-agent-provider
Open

Add Atlas Cloud agent provider#581
binyangzhu000-sudo wants to merge 3 commits into
EtienneLescot:nextfrom
binyangzhu000-sudo:codex/add-atlas-cloud-agent-provider

Conversation

@binyangzhu000-sudo

@binyangzhu000-sudo binyangzhu000-sudo commented Jul 25, 2026

Copy link
Copy Markdown

What does this PR do?

Adds Atlas Cloud as an Agent Workbench LLM provider using the existing OpenAI-compatible ChatOpenAI runtime path.

  • registers the atlascloud provider with atlas / atlas-cloud aliases
  • uses https://api.atlascloud.ai/v1 and deepseek-ai/deepseek-v4-pro defaults
  • supports ATLASCLOUD_API_KEY / ATLAS_CLOUD_API_KEY and Atlas base URL aliases
  • discovers Text models from the Atlas Cloud model catalog
  • keeps disconnected providers disabled for environment fallback at runtime
  • adds offline provider registration/catalog tests and Atlas entries to live provider integration matrices

Follow-up after review

  • moved provider env/base URL keys and Atlas constants into shared provider settings helpers
  • made runtime env fallback honor disabled providers
  • made Atlas Cloud runtime/discovery base URL resolution use explicit config first, then env aliases, then the default Atlas endpoint
  • sends Authorization to Atlas model discovery when an API key is available, while still supporting the public catalog path
  • added Atlas Cloud to the VS Code n8n.agent.provider enum and provider stream integration runner
  • strengthened Atlas provider unit coverage for catalog URL derivation, text model filtering, model over id, dedupe/sort, env precedence, and trailing-slash trimming

Related issue (if any): #

Validation:

  • npm test from packages/vscode-extension (147 passed)
  • npm run build from packages/vscode-extension
  • N8N_AGENT_TEST_PROVIDERS=atlascloud npx tsx --test packages/vscode-extension/tests/integration/provider-stream-v3.integration.test.ts (PROVIDER_STREAM_OK)
  • npm run docs:api -- --dry-run
  • npm run typecheck from docs after installing docs dependencies locally
  • git diff --check

Notes:

  • root npm run build still stops before project compilation at the upstream lockfile supply-chain policy check: xlsx@0.20.2 has no tarball integrity entry. I did not modify pnpm-lock.yaml.
  • local docs build still fails on existing Docusaurus/Mermaid SSR pages with useColorMode is called outside the <ColorModeProvider> for contribution docs paths; this is unrelated to the Atlas provider files changed here.
  • Atlas Cloud reasoning controls are left disabled for now because changing include_reasoning/reasoning payloads would alter the runtime request contract and should be handled as a separate provider capability follow-up once the endpoint contract is confirmed.

@coderabbitai

coderabbitai Bot commented Jul 25, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: c86bbd25-6101-46f5-96bf-6f9215c97bf5

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@EtienneLescot EtienneLescot left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Thanks for this — the Atlas registration itself is clean and consistent with how openrouter is modelled, and it lands in every union/registry that matters (AgentCapabilityProvider, AgentModelProvider, AgentProviderId, the runtime AGENT_MODEL_PROVIDERS) plus all three normalizeAgentProviderId copies. I ran tsx --test tests/unit/agent-provider-atlascloud.test.ts locally: 3/3 pass.

Requesting changes on one item only: the PR also changes credential resolution for every existing provider, and that part has a regression.

Blocking

readProviderEnvironmentSecret bypasses the disabled-providers gate. The runtime now falls back to env keys for the actual API key, but unlike AgentProviderService.readEnvironmentCredential (agent-provider-service.ts:464) it never consults n8n.agent.disabledProviders. Concretely: OPENAI_API_KEY is in the environment, user hits Disconnect on OpenAI → the secret is deleted, openai is added to the disabled set, and listProviderConnectionStates reports connected: false. Before this PR the runtime refused with "Missing API key for OpenAI API"; after it, getProviderRuntimeConfig returns ready: true and the workbench silently bills against the env key the user just explicitly disabled. Same for anthropic / google / mistral / openrouter. Details inline.

The broader env fallback is a reasonable improvement in itself (the provider service already treats env credentials as "connected") — it just needs the same gate, and it deserves a line in the PR description since it is not Atlas-specific.

Non-blocking, worth folding in

  • The controller's envKeys map is a third copy of the credential table and is still missing minimax / minimax-token-plan / copilot-proxy. agent-provider-settings.ts is imported by both files and would be a cycle-free home for it.
  • n8n.agent.provider enum in packages/vscode-extension/package.json:316 lists all ten existing providers but not atlascloud. Nothing reads it (the value lives in globalState), so it is cosmetic — but it is the one place a reader looks for the supported set.
  • scripts/run-provider-stream-v3-integrations.mjs:8 keeps its own provider list and drives the matrix via N8N_AGENT_TEST_PROVIDERS per provider. Atlas was added to the four test files but not here, so npm run test:integration:providers will never exercise it.

Two discussion threads inline: one asking for a live end-to-end pass before merge, one on reasoning effort for the DeepSeek default.

const apiKey = await this._context.secrets.get(getAgentProviderSecretKey(normalizedProvider));
const baseUrl = this.resolveProviderRuntimeBaseUrl(normalizedProvider, settings.baseUrl);
const apiKey = await this._context.secrets.get(getAgentProviderSecretKey(normalizedProvider))
|| this.readProviderEnvironmentSecret(normalizedProvider);

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Blocking. This env fallback ignores the disabled-providers state that the rest of the credential surface honours.

AgentProviderService.readEnvironmentCredential guards with if (this.getDisabledProviders().has(provider)) return undefined; (agent-provider-service.ts:464), and disconnectProvider deletes the stored secret and adds the provider to n8n.agent.disabledProviders. readProviderEnvironmentSecret has no such guard, so after a Disconnect the UI shows connected: false while the runtime happily runs on the env key.

Repro: set OPENAI_API_KEY, Disconnect OpenAI in Settings > Agent Providers, then send a workbench prompt. Pre-PR: "Missing API key for OpenAI API". Post-PR: the run succeeds.

The controller already holds _context.globalState, so the smallest fix is to read DISABLED_PROVIDERS_STATE_KEY here and return undefined for a disabled provider — or to route the lookup through AgentProviderService so there is one code path. A unit test asserting "disabled provider does not pick up its env key" would lock it down.

google: ['GOOGLE_GENERATIVE_AI_API_KEY', 'GEMINI_API_KEY', 'GOOGLE_API_KEY', 'GEMINI_LLM_API_KEY', 'GOOGLE_LLM_API_KEY'],
mistral: ['MISTRAL_API_KEY', 'MISTRAL_LLM_API_KEY'],
openrouter: ['OPENROUTER_API_KEY', 'OPENROUTER_LLM_API_KEY'],
atlascloud: ['ATLASCLOUD_API_KEY', 'ATLAS_CLOUD_API_KEY'],

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

This map is a second source of truth for AGENT_PROVIDER_DEFINITIONS[].envKeys. The PR fixes the existing drift for five providers (good catch on the *_LLM_API_KEY aliases), but then leaves new drift: minimax (MINIMAX_API_KEY), minimax-token-plan (MINIMAX_TOKEN_PLAN_API_KEY) and copilot-proxy (COPILOT_GITHUB_TOKEN, GH_TOKEN, GITHUB_TOKEN) are absent, so env credentials now work for some api-key providers and silently not for others.

agent-provider-service.ts imports from this file, so importing AGENT_PROVIDER_DEFINITIONS back would cycle. agent-provider-settings.ts is already imported by both and would be a cycle-free home for the shared table.


private resolveProviderRuntimeBaseUrl(provider: string, configuredBaseUrl?: string): string | undefined {
if (provider === 'atlascloud') {
return this.readFirstEnvironmentValue(['ATLASCLOUD_BASE_URL', 'ATLAS_CLOUD_BASE_URL', 'ATLASCLOUD_API_BASE', 'ATLAS_CLOUD_API_BASE'])

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Two things here:

  1. configuredBaseUrl is dropped entirely for atlascloud, and env ranks above explicit config — the inverse of every other provider (providerConfig.baseUrl || default). Not reachable through the UI today since providerNeedsBaseUrlInput is openai-compatible-only, but it is a trap for whoever wires up per-provider base URLs later. configuredBaseUrl || env || ATLAS_CLOUD_DEFAULT_BASE_URL keeps the precedence people expect.
  2. This alias list is written twice — here and as baseUrlEnvKeys in AGENT_PROVIDER_DEFINITIONS.atlascloud. Same shared-table point as the env keys above.

: provider === 'google'
? providerConfig.baseUrl || 'https://generativelanguage.googleapis.com/v1beta/openai'
: providerConfig.baseUrl;
: provider === 'atlascloud'

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Nit: unreachable in practice. resolveProviderRuntimeBaseUrl never returns a falsy value for atlascloud (env or default), so providerConfig.baseUrl is always set by the time it gets here. Harmless as defence in depth, but it is the third place https://api.atlascloud.ai/v1 is hardcoded (here, ATLAS_CLOUD_DEFAULT_BASE_URL in this file, and defaultBaseUrl in the provider definitions) — worth collapsing to one exported constant. Fixing the precedence in resolveProviderRuntimeBaseUrl also makes this branch consistent rather than redundant.

const baseUrl = provider === 'openai-compatible' ? configuredBaseUrl : this.readEnvironmentBaseUrl(provider) || definition.defaultBaseUrl;

if ((definition.requiresApiKey || provider !== 'openai-compatible') && !apiKey && definition.authKind !== 'none') {
if (provider !== 'atlascloud' && (definition.requiresApiKey || provider !== 'openai-compatible') && !apiKey && definition.authKind !== 'none') {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

This condition was already dense; the provider !== 'atlascloud' prefix makes the intent hard to recover. A named set reads better and documents why:

const PROVIDERS_WITH_PUBLIC_MODEL_CATALOG = new Set<AgentModelProvider>(['atlascloud']);

Side effects of the bypass worth a thought: a user with no Atlas key can now pick a model from a live list and only discover at run time that requiresApiKey: true blocks the run, and discovery also fires for an Atlas provider the user explicitly disconnected (user-initiated, so not a beacon — but inconsistent with readEnvironmentCredential's disabled gate).

return this.fetchJsonModels(modelsUrl, apiKey ? { Authorization: `Bearer ${apiKey}` } : {});
}

private async fetchAtlasCloudModels(): Promise<string[]> {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

This ignores both apiKey and the baseUrl computed at line 416, so an ATLASCLOUD_BASE_URL override (a self-hosted or regional gateway) still queries api.atlascloud.ai for its model list. Passing the resolved base URL, and an Authorization header when a key exists, costs nothing and keeps the override honest.

No try/catch or timeout is fine — that matches fetchJsonModels, and both call sites already use .catch(() => []).

This is the only genuinely new logic in the PR and it is the one thing not covered by a behavioural test: the type === 'text' filter, model-over-id preference, dedupe, sort, and !response.ok → []. A stubbed fetch in the new unit file would cover all five cheaply.

description: 'ATLASCLOUD_API_KEY',
defaultModel: 'deepseek-ai/deepseek-v4-pro',
defaultBaseUrl: ATLAS_CLOUD_DEFAULT_BASE_URL,
baseUrlEnvKeys: ['ATLASCLOUD_BASE_URL', 'ATLAS_CLOUD_BASE_URL', 'ATLASCLOUD_API_BASE', 'ATLAS_CLOUD_API_BASE'],

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Adding baseUrlEnvKeys to the shared definition shape is the right move — but then resolveProviderRuntimeBaseUrl in agent-runtime-controller.ts hardcodes the same four names instead of reading them from here. Please make one of the two the source of truth. Also worth a unit test for the precedence and the trailing-slash trim in readEnvironmentBaseUrl.

id: 'atlascloud',
label: 'Atlas Cloud',
envKeys: ['ATLASCLOUD_API_KEY', 'ATLAS_CLOUD_API_KEY'],
model: process.env.ATLASCLOUD_MODEL || process.env.ATLAS_CLOUD_MODEL || process.env.N8N_AGENT_TEST_ATLASCLOUD_MODEL || 'deepseek-ai/deepseek-v4-pro',

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Could you run one live end-to-end pass against Atlas before merge, and paste the output here?

N8N_AGENT_TEST_PROVIDERS=atlascloud npx tsx --test packages/vscode-extension/tests/integration/provider-stream-v3.integration.test.ts

The validation notes in the description confirm the model catalog returns 437/121 models, which exercises discovery — but nothing in the PR exercises an actual agent run. That is the part that tends to break for a new OpenAI-compatible endpoint, and the workbench depends on it:

  • tool-calling round-trips through the shim (the whole point of the provider),
  • reasoning content leaking into assistant text — deepseek-ai/deepseek-v4-pro is a thinking model reached through an OpenAI-compatible shim, and agent-runtime-controller has middleware specifically for non-final assistant phases,
  • streaming vs tool-call interaction, cf. the google-specific shouldDisableModelStreamingForToolCalling.

Ideally also tests/integration/tool-usage-workflow-authoring.integration.test.ts and native-mcp-agent-routing.integration.test.ts, since you added Atlas cases to those matrices too. Note that scripts/run-provider-stream-v3-integrations.mjs has its own provider list and was not updated, so the orchestrator will skip Atlas — hence the manual N8N_AGENT_TEST_PROVIDERS above.

});

test('Atlas Cloud provider does not opt into provider-specific reasoning knobs', () => {
assert.equal(getReasoningCapability('atlascloud', 'deepseek-ai/deepseek-v4-pro').supported, false);

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Worth opening this up rather than just asserting the status quo: deepseek-ai/deepseek-v4-pro is the default model and, as a DeepSeek reasoning model, it does have a thinking budget — so pinning supported: false means Workbench users get whatever the endpoint defaults to, with no UI control and no include_reasoning handling.

Atlas exposes an OpenAI-compatible surface, so the existing openrouter-reasoning strategy in agent-provider-capabilities.ts is probably a drop-in:

modelKwargs: { reasoning: { effort }, include_reasoning: true }

Two questions:

  1. Does the Atlas /v1/chat/completions endpoint accept OpenAI-style reasoning: { effort } (or DeepSeek-style thinking) for the DeepSeek models, and does it echo reasoning back in a separate field or inline in content?
  2. If it does not, is false here a deliberate "ship the plumbing first, reasoning later" call?

Either answer is fine by me — I would just rather it be a documented decision than an accident, since it also determines whether reasoning text ends up rendered as assistant output. A follow-up issue is enough if the answer is (2).

assert.ok(providerService.includes("'ATLASCLOUD_API_KEY', 'ATLAS_CLOUD_API_KEY'"), 'Provider service must support both Atlas API key env names');
assert.ok(providerService.includes("normalized === 'atlas' || normalized === 'atlas-cloud'"), 'Provider service must normalize Atlas aliases');

assert.ok(runtimeController.includes("'atlascloud'"), 'Runtime registry must include the Atlas provider id');

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Source-text assertions are an established pattern in this repo (agent-workbench-html.test.ts:222 does the same), so no objection in principle. But this one passes on any occurrence of 'atlascloud' anywhere in a ~4900-line file, so it asserts close to nothing — the display-name and alias assertions below it already cover the intent. The alias test above and the getReasoningCapability assertion are the valuable ones here.

The coverage I would rather have in exchange: fetchAtlasCloudModels payload handling (see the comment on agent-provider-service.ts), readEnvironmentBaseUrl precedence, and the disabled-provider case for the new env-secret fallback.

@binyangzhu000-sudo

Copy link
Copy Markdown
Author

Thanks for the detailed review. I pushed follow-up commit e1993b5f addressing the requested provider fallback hardening:

  • shared provider env/base URL keys now live in agent-provider-settings.ts
  • runtime env fallback now respects n8n.agent.disabledProviders, so disconnected providers no longer pick up env keys
  • Atlas Cloud runtime/discovery base URL resolution now uses explicit config first, then env aliases, then the default endpoint
  • Atlas model discovery sends Authorization when a key is available, still supports the public catalog path, and has helper coverage for text filtering, model over id, dedupe, and sorting
  • added atlascloud to the VS Code provider enum and provider stream integration runner

Validation run:

  • npm test in packages/vscode-extension (146 passed)
  • npm run build in packages/vscode-extension
  • N8N_AGENT_TEST_PROVIDERS=atlascloud npx tsx --test packages/vscode-extension/tests/integration/provider-stream-v3.integration.test.ts (PROVIDER_STREAM_OK)
  • npm run docs:api -- --dry-run
  • npm run typecheck in docs after installing docs dependencies locally
  • git diff --check

I also tried the broader local checks. Root npm run build currently stops at the existing lockfile policy check for xlsx@0.20.2 missing tarball integrity, before project compilation. Local docs build still fails on existing Docusaurus/Mermaid SSR pages with useColorMode is called outside the <ColorModeProvider> on contribution docs paths; this is outside the Atlas provider changes.

@binyangzhu000-sudo

Copy link
Copy Markdown
Author

Small follow-up pushed in 2b3a3780 to strengthen the Atlas Cloud unit coverage requested in review. The Atlas test now checks the shared runtime env key path, disabled-provider fallback guard, catalog URL derivation from an overridden base URL, text-model filtering, model over id, dedupe/sort, and env precedence/trailing-slash trimming.

Re-ran npm test in packages/vscode-extension: 147 passed. git diff --check still passes.

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