fix: clear provider guidance on praisonai --init when no LLM key set#2338
Conversation
fixes #90) Non-programmer users running `praisonai --init` with no API key (or a non-OpenAI key for the OpenAI-default model) previously got a raw stack trace, reinforcing the impression that PraisonAI is OpenAI-only. Add a pre-flight credential check that emits an actionable message pointing at the existing interactive `praisonai setup` wizard (no shell `export` needed) and lists supported providers. Co-authored-by: Mervin Praison <MervinPraison@users.noreply.github.com>
|
@coderabbitai review |
|
/review |
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
✅ Action performedReview finished.
|
Greptile SummaryThis PR adds clearer provider setup handling for
Confidence Score: 4/5This is close, but the provider guard should be fixed before merging.
src/praisonai/praisonai/cli/main.py and the credential provider mapping Important Files Changed
Reviews (2): Last reviewed commit: "fix: align --init provider preflight wit..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
Important Review skippedBot user detected. To trigger a single review, invoke the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe CLI init flow now checks for configured LLM provider credentials before generating agents. If none are detected, it prints setup guidance and exits early. New unit tests cover missing credentials, configured credentials, and credential-check errors. ChangesProvider pre-flight guard
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
|
@copilot Do a thorough review of this PR. Read ALL existing reviewer comments above from Qodo, Coderabbit, and Gemini first — incorporate their findings. Review areas:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/praisonai/tests/unit/cli/test_init_provider_guard.py (1)
20-43: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd one test for the actual
--initearly-return path.These cases only exercise
_provider_preflight_message(). If Line 784's guard is removed or moved after_get_auto_generator(), this file still passes whilepraisonai --initregresses. Please add onePraisonAI.main()test that forcesargs.init, returns a preflight message, and asserts the generator is never reached.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/praisonai/tests/unit/cli/test_init_provider_guard.py` around lines 20 - 43, Add a test for the real `PraisonAI.main()` `--init` early-return path, because the existing `_provider_preflight_message()` unit tests do not cover the guard in `main`. In the new test, force `args.init` to be true and mock `_provider_preflight_message()` to return a message, then assert `PraisonAI.main()` returns early with that message and does not call `_get_auto_generator()`. Use the existing `PraisonAI.main`, `_provider_preflight_message`, and `_get_auto_generator` symbols so the test stays aligned even if the guard moves.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/praisonai/tests/unit/cli/test_init_provider_guard.py`:
- Around line 20-43: Add a test for the real `PraisonAI.main()` `--init`
early-return path, because the existing `_provider_preflight_message()` unit
tests do not cover the guard in `main`. In the new test, force `args.init` to be
true and mock `_provider_preflight_message()` to return a message, then assert
`PraisonAI.main()` returns early with that message and does not call
`_get_auto_generator()`. Use the existing `PraisonAI.main`,
`_provider_preflight_message`, and `_get_auto_generator` symbols so the test
stays aligned even if the guard moves.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e4684466-1d0e-4abd-af7a-1aeb5d06821c
📒 Files selected for processing (2)
src/praisonai/praisonai/cli/main.pysrc/praisonai/tests/unit/cli/test_init_provider_guard.py
|
@claude You are the FINAL architecture reviewer. If the branch is under MervinPraison/PraisonAI (not a fork), you are able to make modifications to this branch and push directly. SCOPE: Focus ONLY on Python packages (praisonaiagents, praisonai). Do NOT modify praisonai-rust or praisonai-ts. CodeRabbit: 2 comments | Qodo: 1 comments | Greptile: 1 comments. Review Context: 7 total comments, 4 bot reviews. Phase 1: Review per AGENTS.md principles (protocol-driven, backward compatible, no perf impact, SDK value gate — no feature bloat). |
|
Claude finished @MervinPraison's task —— View job Final Architecture Review — PR #2338 ✅
Phase 1 — Architecture (AGENTS.md)
Phase 2 — Fixes appliedI confirmed both Greptile P1s were real mismatches between the gate and runtime, and fixed them:
Files modified
Skipped: CodeRabbit's docstring-coverage warning — the new test functions are intentionally self-documenting via comments; adding boilerplate docstrings would be noise (out of scope for a bug fix). Test resultsPhase 3 — Verdict: ✅ Approve (after these fixes)Minimal, protocol-driven, backward-compatible. The gate now provably agrees with runtime resolution, closing both P1 mismatches. No bloat, no new deps, no core/Agent changes. |
…solution Address Greptile P1 findings: the preflight gate could disagree with what AutoGenerator actually does at runtime, so a user could pass the guard and still hit a raw provider auth error. - Inject stored credentials into env before gating so keys saved via `praisonai setup` reach the env-only AutoGenerator resolver. - Gate on the exact runtime model (MODEL_NAME / OPENAI_MODEL_NAME) so a stale OpenAI override with only a non-OpenAI key is caught with guidance instead of failing later. - Add regression tests: stored-credential injection, runtime model override gating, and the real PraisonAI.main() --init early-return path asserting the generator is never constructed. Co-authored-by: Mervin Praison <MervinPraison@users.noreply.github.com>
|
@claude You are the FINAL architecture reviewer. If the branch is under MervinPraison/PraisonAI (not a fork), you are able to make modifications to this branch and push directly. SCOPE: Focus ONLY on Python packages (praisonaiagents, praisonai). Do NOT modify praisonai-rust or praisonai-ts. Read ALL comments above from Gemini, Qodo, CodeRabbit, and Copilot carefully before responding. Phase 1: Review per AGENTS.md
Phase 2: FIX Valid Issues Phase 3: Final Verdict |
|
Claude finished @MervinPraison's task —— View job Final Architecture Review — PR #2338 ✅
Phase 1 — Architecture (AGENTS.md)
Phase 2 — Prior reviewer findings (all already fixed on
|
| runtime_model = _os.environ.get("MODEL_NAME") or _os.environ.get( | ||
| "OPENAI_MODEL_NAME" | ||
| ) | ||
| if is_configured(model=runtime_model): |
There was a problem hiding this comment.
This check still disagrees with the runtime path for OpenRouter models. resolve_llm_endpoint() treats openrouter/... as requiring OPENROUTER_API_KEY, but the credential guard path does not map OpenRouter for model-scoped checks or stored credential injection. When MODEL_NAME=openrouter/... is set and the key was saved through praisonai setup, this guard can proceed while the later generator path never exports OPENROUTER_API_KEY, so --init can still end in the raw provider auth failure instead of the setup guidance.
|
MERGE_GATE_VERDICT: APPROVE Automated fallback — Claude assess did not post a verdict comment (e.g. GitHub MCP unavailable). |
1 participant
Fixes #90
Summary
Addresses the long-standing feedback that PraisonAI onboarding feels OpenAI-only. Most of the issue is already resolved on
main; this PR closes the one remaining beginner-UX gap (acceptance criterion #1).Audit findings (2026-06-26)
llm/env.py): OpenAI, Anthropic, Google/Gemini, Groq, Cohere, Ollama, OpenRouter + 100+ via LiteLLM.auto.pyusesrole_details.get('tools', [])(no hardcoded['']).praisonai setup(provider picker + hidden API-key prompt + unified~/.praisonai/config.yaml), no shellexportrequired.--initflow called the LLM immediately without checking for a configured provider, so a user with no key got a raw stack trace instead of guidance.Change
_provider_preflight_message()incli/main.pythat uses the existing provider-awareis_configured()gate.praisonai --initnow prints a clear, actionable message (pointing atpraisonai setupand listing supported providers) when no provider credential is found, instead of crashing.Noneso a configured user is never blocked.Test plan
tests/unit/cli/test_init_provider_guard.py(3 tests): unconfigured → message; configured →None; internal error →None.tests/unit/llm/test_credentials_is_configured.py(25 tests) still pass.Routing / architecture
praisonai/CLI). No new dependencies, noAgentparams added, no core SDK changes. praisonai-rust / praisonai-ts untouched.🤖 Generated with Claude Code
Summary by CodeRabbit
initflow to check for required AI provider setup before generating agents.