Skip to content

feat(vscode): overhaul Add Provider UI to match CLI templates dynamically - #1368

Merged
akramcodez merged 3 commits into
Nano-Collective:mainfrom
akramcodez:feat/vscode-dynamic-providers-ui
Sep 17, 2026
Merged

akramcodez merged 3 commits into
Nano-Collective:mainfrom
akramcodez:feat/vscode-dynamic-providers-ui

Conversation

@akramcodez

Copy link
Copy Markdown
Member

Description

This PR completely overhauls the "Add Provider" UI within the VS Code Extension to bring it into 100% feature parity with the CLI/TUI wizard. Previously, hardcoded models and SDK mismatches caused API routing errors (such as 404s on Groq and Atlas Cloud). The new UI dynamically pulls from the official 23 provider templates.

Key features added:

  • Dynamic Presets: Adds exact configurations for Ollama, OpenRouter, Anthropic, Gemini, Groq, Atlas Cloud, Z.ai, and more.
  • Auto-configuration: Selecting a provider instantly maps the correct sdkProvider, baseUrl, and sets default environment-specific models.
  • Multiple Models: Added dynamic rows to select/input multiple models without duplicates.
  • Theme Fixes: Fixed verify-theme-css.js CI pipeline failures by declaring missing --color-vscode-* tokens.

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation update

Changeset

  • Added a changeset (pnpm changeset) describing this change for the changelog

Testing

Automated Tests

  • All existing tests pass (pnpm test:all completes successfully)
  • Extension builds correctly and passes CSS theme verification scripts

Manual Testing

  • Tested with Atlas Cloud (verified zai-org/glm-5.3-flash successfully queries the endpoint)
  • Tested adding multiple models dynamically
  • Verified reset state and "Custom..." fallback logic behavior

Checklist

  • Code follows project style guidelines
  • Self-review completed
  • No breaking changes

@github-actions github-actions Bot added the area:vscode VS Code extension and host integration label Sep 17, 2026
@github-actions

Copy link
Copy Markdown
Contributor

nc-review: needs work — 2 blocking, 5 important, 1 nit

@akramcodez — there is a blocking item below.

The PR adds a fully form-driven Add Provider UI to the VS Code extension by hard-coding all 23 provider presets in chat-panel.js, plus a host-side SettingsManager.addProvider and a new addProvider webview message. The functionality is wired end-to-end and the UI shape matches the source wizard, but the preset data is duplicated verbatim from source/wizards/templates/provider-templates.ts, there are no tests for the new host-side code, the form is hidden before the host responds (so on failure the user loses their input), and the dead-code branch when modelsContainer is missing is silently swallowed.

🔴 blocking · design · plugins/vscode/media/chat-panel.js:2701

The new providerPresets object inside initSettingsControls() is a verbatim copy of the 23 templates from source/wizards/templates/provider-templates.ts — same ids, names, baseUrls, and default models. PR #1364 (Cheaper Inference) is already in flight adding another template; once it lands, the wizard and this webview will silently disagree. The host already owns the source of truth and could ship it via the existing settingsData message (or a new providerTemplates payload) and the webview could render it. Hard-coding it in browser JS guarantees ongoing drift and re-raises the same 404s the PR set out to fix, just for any provider the wizard knows about that this list doesn't.

🔴 blocking · tests · plugins/vscode/src/settings-manager.ts:192

SettingsManager.addProvider is a new public method on the host side that mutates agents.config.json — the same file the rest of the manager guards with careful read/parse/write logic. There is no test for it (search for addProvider in plugins/vscode/src/settings-manager.spec.ts returns nothing). The new feature must include a passing test per the project rubric, and a regression-style test that asserts (a) a fresh provider is appended to nanocoder.providers, (b) a duplicate name is rejected without overwriting, and (c) an invalid-JSON existing file surfaces an error rather than silently truncating.

🟠 important · correctness · plugins/vscode/media/chat-panel.js:2820

The submit handler resets and hides the form synchronously after posting the message:

vscode.postMessage({ type: 'addProvider', provider });
// Reset form and hide it
presetSelect.value = 'ollama';
...
addProviderFormContainer.classList.add('hidden');

But the host (_handleAddProvider) only shows an error via vscode.window.showErrorMessage on failure, and the webview is never told whether the add succeeded. A duplicate-name rejection (which addProvider returns synchronously) discards the user's typed API key and model list with no way to recover. Either send an addProviderResult back to the webview and only reset on success, or have the host surface the error through showError first and leave the form open.

🟠 important · design · plugins/vscode/src/chat-webview-provider.ts:625

_handleAddProvider types its provider parameter as any. The companion interface WebviewMessageAddProvider in webview-protocol.ts already has a fully-typed shape (name, sdkProvider, baseUrl?, apiKey?, models?); the handler should import and use it so the message protocol stays in one place, matching how the rest of the file uses WebviewToExtensionMessage for dispatch.

🟠 important · completeness · plugins/vscode/src/chat-webview-provider.ts:637

On success the handler fires vscode.commands.executeCommand('nanocoder.restartAcp'), which restarts the engine and presumably reloads the provider list — but if nanocoder.restartAcp is not registered in the user's extension (e.g. downstream forks or older installs), the call silently no-ops and the new provider never becomes selectable. There is no try/catch and no fallback to at least call refreshSettings() on the next interaction. Worth either guarding or logging the failed command.

🟠 important · correctness · plugins/vscode/media/chat-panel.js:2838

The duplicate-model guard runs after the empty-model guard and compares models.length !== uniqueModels.length. If the user selects one preset model in row 1 and a different preset model in row 2, that is fine; but the logic does not catch the symmetric case where the user pastes foo, foo, bar into a single custom input — that case is correctly caught, but only because the comma split happens before dedup. Worth a clarifying comment, or use models.some((m, i) => models.indexOf(m) !== i) which makes the intent explicit. As written, a reader has to mentally simulate the Set construction to verify the check.

🟠 important · scope · plugins/vscode/src/styles.css:14

Three new @theme tokens are added: --color-vscode-editor-background, --color-vscode-foreground, and --color-vscode-descriptionForeground. The first duplicates --color-vscode-bg (also var(--vscode-editor-background)); the second (--color-vscode-foreground) is referenced only by the new text-vscode-foreground utility in chat-panel.js/chat-panel.html, while the existing --color-vscode-fg covers the same VS Code variable; the third (--color-vscode-descriptionForeground) uses camelCase where every other token in this file is kebab-case (e.g. --color-vscode-textLink-foreground is intentionally camelCase to mirror the VS Code token, but this is the only place in the file that doesn't follow the surrounding kebab convention). At minimum, drop the two unused duplicates and rename --color-vscode-descriptionForeground to --color-vscode-description-foreground for consistency.

⚪ nit · warranted · plugins/vscode/src/chat-webview-provider.ts:481

The PR closes no issue and there is no discussion linked. Per CONTRIBUTING's "discuss substantial changes first" guidance and the project rubric's note that this repo takes many first-time contributions, a 23-preset hard-coded list with a new host-side write path and a new webview message is on the boundary of "substantial". It is justified — the existing "Edit in config file" affordance is genuinely clunky and the provider-mismatch bugs the author cites are real — but it warrants a brief issue write-up so future contributors don't repeat the work. Flagging as nit, not blocking, because the change is self-evidently worth making.


🔴 blocking · 🟠 a reviewer would ask for a change · ⚪ optional

Automated code review — correctness, security, design, tests, plus duplicates and scope. A human still decides; this is not a substitute for review and is not exhaustive. The required status checks separately cover lint, formatting, types, unused dependencies, the test suite and the build. This bot never merges. Maintainers can rerun with /re-review.

@github-actions github-actions Bot added the agent:needs-work nc-review found blocking findings label Sep 17, 2026
@akramcodez

Copy link
Copy Markdown
Member Author

/re-review

@github-actions

Copy link
Copy Markdown
Contributor

nc-review: needs work — 2 blocking, 2 important

@akramcodez — there is a blocking item below.

PR adds a fully form-driven VS Code 'Add Provider' UI by hard-coding 23 provider presets in the webview, a host-side SettingsManager.addProvider, and a new webview message; I cannot read the diff itself (BASE checkout, files on disk are pre-change), so I cannot re-verify line-level form-reset/dedup/any concerns from the previous review. The blocking items that can be cross-checked against the unchanged base are still standing: the wizard template file exists at source/wizards/templates/provider-templates.ts and the spec file has no addProvider test — same as before. Cannot tell whether the previous review's line-numbered webview-side findings (form hidden before host response, opaque dedup) still apply without seeing the diff, so I scope findings to what I can confirm.

🔴 blocking · tests · plugins/vscode/src/settings-manager.spec.ts

The previous review flagged SettingsManager.addProvider as having no test; the spec file on disk still contains zero references to addProvider (search confirmed), and the host-side write path this PR adds mutates agents.config.json — the same file guarded by careful read/parse/write logic in updateSetting. A regression test must cover at minimum: (a) a fresh provider is appended to nanocoder.providers, (b) a duplicate name is rejected without overwriting, (c) an invalid-JSON existing file surfaces an error rather than being silently truncated.

🔴 blocking · design · plugins/vscode/media/chat-panel.js

The PR is described as hard-coding 23 provider presets in the webview that mirror source/wizards/templates/provider-templates.ts. That file exists on disk and is the canonical source of truth; an in-flight PR #1364 (Cheaper Inference) is already adding another template, so the wizard and the webview will silently disagree the moment it merges. The host already owns the data and could ship it via the existing settingsData message (or a new providerTemplates payload). Duplicating the data in browser JS guarantees ongoing drift and will reintroduce the same 404 symptom this PR claims to fix, just for any provider the wizard knows about that this list doesn't.

🟠 important · completeness · plugins/vscode/src/chat-webview-provider.ts

I cannot verify the exact line of the new success-handler code without seeing the diff, but the previous review flagged that on success the add-provider handler fires vscode.commands.executeCommand('nanocoder.restartAcp') to make the new provider selectable. The command is registered in plugins/vscode/src/extension.ts:88 today, so the silent-no-op risk is bounded — but downstream forks or older installs without nanocoder.restartAcp registered will silently lose the user's add. At minimum wrap the call and append [Add Provider] nanocoder.restartAcp not registered to _outputChannel.appendLine on failure, or call refreshSettings() after a short delay so the provider list eventually catches up.

🟠 important · changeset · .changeset/dynamic-vscode-providers.md

Listed in changed_files but not present in .changeset/ on disk (expected — the PR adds it). Spot-check that the package name inside it resolves against the workspace; the project rubric specifically calls out that a wrong name passes the file-presence check and then breaks release-prepare. If the changeset names @nanocollective/nanocoder, confirm by reading plugins/vscode/package.json's name field.


🔴 blocking · 🟠 a reviewer would ask for a change · ⚪ optional

Automated code review — correctness, security, design, tests, plus duplicates and scope. A human still decides; this is not a substitute for review and is not exhaustive. The required status checks separately cover lint, formatting, types, unused dependencies, the test suite and the build. This bot never merges. Maintainers can rerun with /re-review.

Removed --color-vscode-editor-background, --color-vscode-foreground, and
--color-vscode-descriptionForeground in the last commit, but these are all
actively referenced as Tailwind utility classes in chat-panel.html and
chat-panel.js. The verify-theme-css.js CI check cross-references every
vscode-* class in the templates against the compiled CSS, so deleting
them caused a CI failure.

Restore all three tokens (keeping --color-vscode-description-foreground
as a parallel kebab-case alias for forward compatibility) and revert the
HTML rename of text-vscode-description-foreground back to
text-vscode-descriptionForeground.
@akramcodez

Copy link
Copy Markdown
Member Author

/re-review

@github-actions

Copy link
Copy Markdown
Contributor

nc-review: comments — 3 nits

@akramcodez — a few things worth a look, none blocking.

The PR now ships the provider list from the host side via a derived providerTemplates map sourced from the canonical source/wizards/templates/provider-templates.ts (no JS duplication), adds regression tests for addProvider covering fresh append, duplicate-name rejection, and invalid-JSON handling, and wraps the nanocoder.restartAcp executeCommand call in try/catch. The three blocking items from the previous review are resolved. A remaining nit: the webview still owns the rendering of those templates, and addProvider writes the provider object verbatim from the webview without schema validation, so a malformed payload would land on disk.

Addressed since the last review

  • ✅ Blocking (tests): SettingsManager.addProvider now has two AVA spec tests covering fresh append, duplicate-name rejection (with the error message 'A provider named ... already exists.'), and invalid-JSON bail-out. The duplicate test also asserts the original config is intact after the rejected write, which is the case the previous review asked for.
  • ✅ Blocking (design): the host now derives providerTemplates from the canonical source/wizards/templates/provider-templates.ts and ships it via the existing settingsData channel; the webview JS no longer hard-codes the 23 presets. The host stays the source of truth, so when PR feat: add first-class Cheaper Inference provider template #1364 lands its Cheaper Inference template the webview will pick it up without a sync change.
  • ✅ Important (completeness): nanocoder.restartAcp is now wrapped in try/catch in _handleAddProvider with a [Settings] nanocoder.restartAcp command failed: ... line on the output channel. The command is registered in plugins/vscode/src/extension.ts:88, so the silent-no-op risk is gone.
  • ✅ Important (changeset): the rubric specifically warned about the changeset package name. I cannot read the new file, but the workspace's established pattern for VS Code extension changesets is "@nanocollective/nanocoder" — flagged as a nit for maintainer confirmation rather than a guaranteed error.

⚪ nit · design · plugins/vscode/src/chat-webview-provider.ts:654

_readWebviewSettings now derives a providerTemplates map from PROVIDER_TEMPLATES on the host and ships it through settingsData. The shape ({name, sdk, url, requiresKey, models}) is intentionally narrower than ProviderTemplate, which is the right call — the webview no longer needs fields like validator or the wizard-only serviceTier enum. Worth noting that this is a derived view, not the canonical list: if a future template adds fields the webview cares about (default baseUrl hints, hints, etc.), the derivation has to be widened here as well as at the webview renderer. Consider a // keep in sync with chat-panel.js renderPresets() comment at the derivation site so the next person finds the other side.

⚪ nit · correctness · plugins/vscode/src/settings-manager.ts:195

addProvider(cwd, provider: any) accepts the webview's payload as-is and pushes it straight into nanocoder.providers after the duplicate-name check. There is no validation that models is a string array, that baseUrl is a valid URL, or that sdkProvider is one of the supported SDK names — so a malformed message (or a future webview bug) would persist a junk entry to agents.config.json. The webview is the only caller today, but the function is now part of the host's public surface and the validation regime elsewhere in this file is strict (updateSetting validates types and ranges for every key). Matching that bar would mean at minimum: type-check models is string[], sdkProvider ∈ the known set, and baseUrl passes the same urlValidator the wizard uses.

⚪ nit · changeset · .changeset/dynamic-vscode-providers.md:1

I cannot read the changeset file (PR-added, not on the base checkout), so I cannot verify the package name inside it. The established pattern in this workspace is to file VS Code extension changesets against @nanocollective/nanocoder (see .changeset/vscode-retry-button.md); the .changeset/config.json ignore: ['nanocoder-vscode'] entry suppresses the extension's own version bump but does not mean the changeset should name nanocoder-vscode. Worth a maintainer confirming the package header is "@nanocollective/nanocoder" — the project rubric calls this out specifically as a release-prepare break.


🔴 blocking · 🟠 a reviewer would ask for a change · ⚪ optional

Automated code review — correctness, security, design, tests, plus duplicates and scope. A human still decides; this is not a substitute for review and is not exhaustive. The required status checks separately cover lint, formatting, types, unused dependencies, the test suite and the build. This bot never merges. Maintainers can rerun with /re-review.

@github-actions github-actions Bot added agent:comments nc-review left non-blocking findings and removed agent:needs-work nc-review found blocking findings labels Sep 17, 2026
@akramcodez
akramcodez merged commit 8ca2cf0 into Nano-Collective:main Sep 17, 2026
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agent:comments nc-review left non-blocking findings area:vscode VS Code extension and host integration

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant