Skip to content

feat(config): list valid providers in unknown provider error#221

Merged
404-Page-Found merged 1 commit into
404-PF:mainfrom
c436zhan:fix/config-set-list-valid-providers
Jul 13, 2026
Merged

feat(config): list valid providers in unknown provider error#221
404-Page-Found merged 1 commit into
404-PF:mainfrom
c436zhan:fix/config-set-list-valid-providers

Conversation

@c436zhan

@c436zhan c436zhan commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Summary

commit-echo config set provider <key> rejected unknown provider keys with only Unknown provider: <key>., giving users no hint about which providers are available. This aligns the error message with the pattern already used in assertKnownProvider inside the provider layer.

Root cause

parseConfigSetValue in src/commands/config.ts threw a bare error mentioning only the invalid key, while assertKnownProvider in src/providers/index.ts already listed valid options in its error. The two paths were inconsistent.

Changes

  • src/providers/index.ts: export getValidProviderKeys() so it can be reused by the config command.
  • src/commands/config.ts: import getValidProviderKeys and include the valid provider list in the config set provider error message.
  • tests/config-command.test.mjs: extend the existing unknown-provider test to assert that the error output contains Valid providers: and mentions at least openai and anthropic.

Testing

  • npm run build — compiles cleanly
  • node --test tests/config-command.test.mjs — 24 tests, 22 pass (2 pre-existing unrelated JSON-output failures)
  • node --test tests/provider-validation.test.mjs — 3 tests, all pass
  • npm run format:check — no formatting issues in changed files

Compatibility

No breaking changes. The error message now includes extra helpful text; existing scripts that regex-match Unknown provider: will still match.

Fixes #216

Summary by CodeRabbit

  • Bug Fixes

    • Improved configuration errors for unknown providers by displaying the invalid value and a list of valid provider options.
  • Documentation

    • Provider validation details are now clearer when configuring the application.

@c436zhan c436zhan requested a review from 404-Page-Found as a code owner July 13, 2026 02:17
@coderabbitai

coderabbitai Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: a19da990-fb54-4464-b25d-76db1dc49092

📥 Commits

Reviewing files that changed from the base of the PR and between a9fd4c6 and 2243c8d.

📒 Files selected for processing (3)
  • src/commands/config.ts
  • src/providers/index.ts
  • tests/config-command.test.mjs
📜 Recent review details
🧰 Additional context used
📓 Path-based instructions (3)
src/**/*.ts

📄 CodeRabbit inference engine (AGENTS.md)

The project is ESM-only: use .js extensions in local imports and keep verbatimModuleSyntax-compatible TypeScript imports (for example, import type for type-only imports).

Files:

  • src/providers/index.ts
  • src/commands/config.ts
src/providers/**/*.ts

📄 CodeRabbit inference engine (AGENTS.md)

All LLM provider implementations must conform to the shared Provider interface and integrate with the provider abstraction used by the factory/registry system.

Files:

  • src/providers/index.ts
tests/**/*.mjs

📄 CodeRabbit inference engine (AGENTS.md)

tests/**/*.mjs: Use Node.js built-in node:test for test files in tests/ and tests/e2e/; do not use Jest or Mocha.
Write test assertions with node:assert/strict in test files under tests/ and tests/e2e/.

Files:

  • tests/config-command.test.mjs
🔇 Additional comments (3)
src/providers/index.ts (1)

12-12: LGTM!

src/commands/config.ts (1)

11-11: LGTM!

Also applies to: 56-56, 86-86, 120-120

tests/config-command.test.mjs (1)

283-295: LGTM!


📝 Walkthrough

Walkthrough

The config command now lists valid provider keys when rejecting an unknown provider. The provider-key helper is exported for reuse, and the config command test verifies the expanded error message.

Changes

Provider validation

Layer / File(s) Summary
Export valid provider keys
src/providers/index.ts
getValidProviderKeys is exported without changing its behavior.
Improve config provider errors
src/commands/config.ts, tests/config-command.test.mjs
Unknown provider errors include the invalid value and valid provider list; the test verifies openai and anthropic appear. Formatting-only changes are also included.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

Suggested reviewers: 404-page-found

Poem

A bunny found a provider key,
And listed choices clearly.
“No mystery hops,” it said with cheer,
“The valid names are waiting here!”
🐇✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning The PR adds valid provider names to the unknown-provider error and updates tests, but the reported test suite still has failing unrelated cases. Resolve the remaining test failures so the full suite passes, or clarify if they are pre-existing and excluded from this PR.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: improved unknown-provider errors in config set.
Out of Scope Changes check ✅ Passed The changes stay focused on config provider validation, error messaging, and the matching test update.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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.

@404-Page-Found 404-Page-Found 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.

LGTM

@404-Page-Found 404-Page-Found merged commit 705685c into 404-PF:main Jul 13, 2026
1 check passed
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.

List valid providers in config set unknown-provider errors

2 participants