Github actions fix#1
Merged
Merged
Conversation
MervinPraison
added a commit
that referenced
this pull request
Mar 21, 2024
Merge pull request #1 from MervinPraison/develop
MervinPraison
added a commit
that referenced
this pull request
Dec 26, 2025
- Fix #1: MemoryConfig Pydantic validation - add _prepare_mem0_config() to strip PraisonAI-specific reranker fields (enabled, default_rerank) before passing to mem0, which only accepts provider and config fields - Fix #2: LLM Reranker 'Chat' object not callable - check if client.chat is callable, not just if it exists. OpenAI client has chat as namespace object. Also handle max_completion_tokens vs max_tokens for different model versions. - Fix #3: Chroma vector store metadata validation - ensure non-empty metadata dicts by adding placeholder key when metadata is empty or None - Fix test file issues: add user_id parameter (required by mem0 API), handle dict return type from knowledge.search, use gpt-4o-mini for reranker test (gpt-5-nano returns empty responses with max_completion_tokens=10)
This was referenced Mar 30, 2026
2 tasks
This was referenced Apr 6, 2026
praisonai-triage-agent Bot
added a commit
that referenced
this pull request
Apr 8, 2026
This addresses architectural gaps #1-3 by implementing: 1. LLMProviderAdapter protocol to replace scattered provider dispatch 2. UnifiedLLMProtocol to consolidate dual execution paths 3. Memory and Knowledge adapter registries for protocol-driven backends Changes: - Add LLMProviderAdapter and UnifiedLLMProtocol to llm/protocols.py - Create memory/adapters/registry.py with thread-safe adapter registration - Create knowledge/adapters/registry.py with thread-safe adapter registration - Register core adapters (sqlite, in_memory) in core SDK - Heavy implementations (chromadb, mem0) to be moved to wrapper Follows AGENTS.md protocol-driven core principle. Backward compatible. Co-authored-by: MervinPraison <MervinPraison@users.noreply.github.com>
This was referenced Apr 8, 2026
Closed
Core SDK Gap #3: Memory/Knowledge adapter protocols exist but bypassed by core implementations
#1306
Closed
This was referenced Apr 22, 2026
Closed
MervinPraison
pushed a commit
that referenced
this pull request
Apr 28, 2026
…locate smoke Builds on triage-agent commit 79eec46 (which addressed gemini medium and attempted greptile P2 #1) by addressing the still-open findings and fixing two issues introduced by that commit. 1) _async_bridge._spawn_locked: enforce 'caller must hold the lock' invariant via 'assert self._lock.locked()' (greptile P2 #2: 'get_unlocked() is now a transparent alias \u2014 contract is unenforced'). 2) test_main_dispatcher.test_version_does_not_import_typer_app: replace mock.patch('praisonai.cli.app.register_commands') with sys.modules eviction of all praisonai.cli.* modules. mock.patch with a dotted target imports the target module on entry, which defeats the 'no Typer app import on --version' invariant the test claims to verify (coderabbit major; verified via docs.python.org/3/library/unittest.mock.html). 3) test_main_dispatcher.test_{legacy,typer}_path_restores_argv: set sys.argv[0] to a non-'praisonai' value. The dispatcher rewrites sys.argv to ['praisonai'] + sys.argv[1:] inside the try block, so the previous argv values (and the variant in 79eec46 with '--extra-arg') were identical to the rewrite \u2014 the assertion passed regardless of whether 'finally' restored argv. Differing argv[0] makes the assertion catch a missing finally clause (greptile P2 #1: 'finally-restore tests had no discriminating power'). 4) test_async_bridge.py: - Removed the file at repo root (clutter; not under src/praisonai/tests/; uses 'from src.praisonai.praisonai...' source-tree import that won't work for installed users). - Added src/praisonai/tests/unit/test_async_bridge.py as proper unittest.TestCase tests with the helper renamed from 'test_coro' to '_coro' so pytest does not auto-collect it as a test (coderabbit major on the original file). - Added a regression test for the new _spawn_locked() lock assert. - Coroutine .close() on the RuntimeError path so '-W error::RuntimeWarning' stays clean. Verified: 27/27 pass (20 dispatcher + 7 async bridge), no warnings.
MervinPraison
added a commit
that referenced
this pull request
Apr 28, 2026
…patcher (#1575) * fix(wrapper): _async_bridge daemon thread + tests for unified CLI dispatcher Two small wrapper-layer fixes that surfaced during the post-#1548 review: 1) src/praisonai/praisonai/_async_bridge.py - Mark the background event-loop thread as daemon=True so short-lived scripts (CLI commands, smoke tests, single-shot agent invocations) exit cleanly. The previous daemon=False thread, combined with atexit-registered shutdown, allowed the interpreter to hang for the full PRAISONAI_RUN_SYNC_TIMEOUT (default 300s) on exit when the loop's shutdown coroutine couldn't drain. - Drop the atexit registration that races interpreter shutdown. Long-running servers (gateway, a2u, mcp_server) can still call the public shutdown() hook explicitly to cancel in-flight tasks. - DRY the loop+thread bring-up into _spawn_locked() shared by get() and get_unlocked(). - Tighten run_sync(): raise RuntimeError loudly when called from inside a running loop (was previously a dead 'running' var followed by a blocking fut.result that would deadlock the caller's loop). 2) src/praisonai/tests/unit/cli/test_main_dispatcher.py (new, 20 tests) - Cover _is_legacy_invocation: empty/flag/prompt/yaml-existing/yaml- missing/typer-subcommand cases. - Cover --version / -V short-circuit path and verify it does not import the Typer app or legacy runner. - Cover legacy routing for free-text prompts and existing YAML files, including PraisonAI.main() returning False -> exit code 1, and sys.argv restoration after dispatch. - Cover Typer routing for no-args, --help, subcommand, and ImportError during register_commands() (must propagate; fail-loud design). - Cover SystemExit propagation from Typer with non-zero code. All tests run in <0.4s. Smoke test exits cleanly (was hanging until PRAISONAI_RUN_SYNC_TIMEOUT before the daemon=True fix). * fix: address reviewer feedback - Generalize docstring references to avoid maintenance overhead - Improve test coverage for argv restoration by using discriminating values Co-authored-by: Mervin Praison <MervinPraison@users.noreply.github.com> * fix(review): layer remaining greptile P2 + coderabbit major fixes; relocate smoke Builds on triage-agent commit 79eec46 (which addressed gemini medium and attempted greptile P2 #1) by addressing the still-open findings and fixing two issues introduced by that commit. 1) _async_bridge._spawn_locked: enforce 'caller must hold the lock' invariant via 'assert self._lock.locked()' (greptile P2 #2: 'get_unlocked() is now a transparent alias \u2014 contract is unenforced'). 2) test_main_dispatcher.test_version_does_not_import_typer_app: replace mock.patch('praisonai.cli.app.register_commands') with sys.modules eviction of all praisonai.cli.* modules. mock.patch with a dotted target imports the target module on entry, which defeats the 'no Typer app import on --version' invariant the test claims to verify (coderabbit major; verified via docs.python.org/3/library/unittest.mock.html). 3) test_main_dispatcher.test_{legacy,typer}_path_restores_argv: set sys.argv[0] to a non-'praisonai' value. The dispatcher rewrites sys.argv to ['praisonai'] + sys.argv[1:] inside the try block, so the previous argv values (and the variant in 79eec46 with '--extra-arg') were identical to the rewrite \u2014 the assertion passed regardless of whether 'finally' restored argv. Differing argv[0] makes the assertion catch a missing finally clause (greptile P2 #1: 'finally-restore tests had no discriminating power'). 4) test_async_bridge.py: - Removed the file at repo root (clutter; not under src/praisonai/tests/; uses 'from src.praisonai.praisonai...' source-tree import that won't work for installed users). - Added src/praisonai/tests/unit/test_async_bridge.py as proper unittest.TestCase tests with the helper renamed from 'test_coro' to '_coro' so pytest does not auto-collect it as a test (coderabbit major on the original file). - Added a regression test for the new _spawn_locked() lock assert. - Coroutine .close() on the RuntimeError path so '-W error::RuntimeWarning' stays clean. Verified: 27/27 pass (20 dispatcher + 7 async bridge), no warnings. --------- Co-authored-by: MervinPraison <praison@users.noreply.github.com> Co-authored-by: praisonai-triage-agent[bot] <272766704+praisonai-triage-agent[bot]@users.noreply.github.com> Co-authored-by: Mervin Praison <MervinPraison@users.noreply.github.com>
MervinPraison
pushed a commit
that referenced
this pull request
Apr 28, 2026
CodeRabbit Major #1 (cli/app.py partial-registration cache poisoning): register_commands() flipped _commands_registered=True BEFORE running its 50+ command imports and app.add_typer() calls. If any import failed mid-flight the sentinel stayed set, and a later caller would short-circuit and dispatch against a half-built command tree. Move the sentinel assignment to the END of register_commands(), and document why in the docstring + inline comment. (In practice the bug is largely mitigated today by the unconditional register_commands() call at module import time on line 683 — but defense in depth costs nothing and the invariant is now correct.) CodeRabbit Major #2 (_run_typer race against partial registration): _run_typer() called register_commands() WITHOUT holding the new _typer_commands_lock, so a concurrent _get_typer_commands() caller could observe partial state. Hold the lock around register_commands() in _run_typer too. CRITICAL: do NOT route through _get_typer_commands() itself, because that method intentionally swallows exceptions to keep the cache best-effort — wrapping _run_typer in that swallow would silently downgrade real registration errors (e.g. ImportError from a missing optional dep) to Typer's 'no commands' behaviour. Inline comment spells this out. Greptile P2 #1 (test_typer_registration_failure_propagates removed): The deleted test pinned the fail-loud guarantee from #1577's description. Re-introduced as TestTyperRegistrationFailureFailsLoud with two cases (ImportError and RuntimeError both propagate) so a future refactor that wraps register_commands() in defensive try/except will fail CI. Greptile P2 #2 (missing trailing newline at EOF in __main__.py): Trivial; added. Verification: - test_main_dispatcher.py: 28/28 pass (was 26) - test_auto_lazy_loading.py::TestThreadSafety: 5/5 pass - test_async_bridge.py: 7/7 pass - Smoke probe: clean exit, daemon thread invariants intact.
MervinPraison
added a commit
that referenced
this pull request
Apr 28, 2026
…loses #1576) (#1577) * fix(wrapper): restore #1561 auto-discovery dispatcher in __main__.py (closes #1576) When #1548 merged at d0d87a7, the conflict in __main__.py was resolved by taking the PR's heuristic-based dispatcher (_is_legacy_invocation), discarding the strictly-better auto-discovery dispatcher introduced in #1561 (4b1103c). Two pre-existing tests have been red on main since: FAILED test_auto_lazy_loading.py::TestThreadSafety::test_typer_commands_concurrent_calls_all_return_set FAILED test_auto_lazy_loading.py::TestThreadSafety::test_typer_commands_failure_does_not_poison_cache Both fail with: AttributeError: module 'praisonai.__main__' has no attribute '_get_typer_commands'. Restore __main__.py to the version from 4b1103c (#1561). The restored dispatcher reintroduces: - _typer_commands_cache + _typer_commands_lock: thread-safe cached Typer command discovery via Click introspection. - _get_typer_commands(): defensive logger.warning + non-poisoning cache on failure (next caller can retry). - _find_first_command(argv): flag/value-flag aware first-positional finder. - _run_typer / _run_legacy: clear separation of dispatch paths with explicit argv save/restore. - main() routing rules: 1) --version, 2) --help, 3) no argv, 4) first arg in Typer command set, 5) everything else -> legacy. Routing-by-discovered-command-set is strictly better than the heuristic: new Typer commands are picked up automatically with no opt-in, and discovery is fail-loud at dispatch time (register_commands() inside _run_typer) while the cache layer is best-effort. Behavior preserved from #1548: - --version still short-circuits before importing praisonai.cli.* (verified via new sys.modules eviction test). - register_commands() exceptions still propagate from _run_typer (no silent degradation). test_main_dispatcher.py rewritten to cover the restored API: - TestFindFirstCommand (7 tests) - TestGetTyperCommandsCache (4 tests) - TestVersionShortCircuit (3 tests, incl. sys.modules-eviction proof) - TestMainRouting (8 tests) - TestRunLegacyArgvRestoration (3 tests with discriminating argv[0]) - TestRunTyperArgvRestoration (1 test with discriminating argv[0]) Total: 26 tests, all green. Verification: - test_auto_lazy_loading.py::TestThreadSafety: 5/5 pass (was 3/5). - test_main_dispatcher.py: 26/26 pass. - test_async_bridge.py: 7/7 pass. - Smoke probe: clean exit; daemon thread invariants intact. - Broad sweep (1417 tests): 1412 pass / 5 fail / 2 skip. All 5 failures are pre-existing on main, verified by running test_gateway_install_failure on both branches. Fixes #1576. * fix(review): address all reviewer findings on #1577 CodeRabbit Major #1 (cli/app.py partial-registration cache poisoning): register_commands() flipped _commands_registered=True BEFORE running its 50+ command imports and app.add_typer() calls. If any import failed mid-flight the sentinel stayed set, and a later caller would short-circuit and dispatch against a half-built command tree. Move the sentinel assignment to the END of register_commands(), and document why in the docstring + inline comment. (In practice the bug is largely mitigated today by the unconditional register_commands() call at module import time on line 683 — but defense in depth costs nothing and the invariant is now correct.) CodeRabbit Major #2 (_run_typer race against partial registration): _run_typer() called register_commands() WITHOUT holding the new _typer_commands_lock, so a concurrent _get_typer_commands() caller could observe partial state. Hold the lock around register_commands() in _run_typer too. CRITICAL: do NOT route through _get_typer_commands() itself, because that method intentionally swallows exceptions to keep the cache best-effort — wrapping _run_typer in that swallow would silently downgrade real registration errors (e.g. ImportError from a missing optional dep) to Typer's 'no commands' behaviour. Inline comment spells this out. Greptile P2 #1 (test_typer_registration_failure_propagates removed): The deleted test pinned the fail-loud guarantee from #1577's description. Re-introduced as TestTyperRegistrationFailureFailsLoud with two cases (ImportError and RuntimeError both propagate) so a future refactor that wraps register_commands() in defensive try/except will fail CI. Greptile P2 #2 (missing trailing newline at EOF in __main__.py): Trivial; added. Verification: - test_main_dispatcher.py: 28/28 pass (was 26) - test_auto_lazy_loading.py::TestThreadSafety: 5/5 pass - test_async_bridge.py: 7/7 pass - Smoke probe: clean exit, daemon thread invariants intact. --------- Co-authored-by: MervinPraison <praison@users.noreply.github.com>
This was referenced May 10, 2026
MervinPraison
added a commit
that referenced
this pull request
Jun 4, 2026
…on, cli_backend support (#1797) * fix: wrapper layer gaps - InteractiveRuntime lifecycle, tool resolution, cli_backend support Fixes #1646 - Issue #1: Fixed InteractiveRuntime lifecycle bug where runtime was stopped before agents.start() * Removed premature stop() in inner finally block at lines 1234-1240 * Added proper loop closure check in cleanup block * Runtime now stays alive until after agent execution completes - Issue #2: Consolidated tool resolution implementations * Added instantiate=True parameter to ToolResolver.resolve() for automatic class instantiation * Removed hardcoded standard_tools whitelist in agents_generator.py * Replaced custom tool resolution in bots_cli.py and job_workflow.py with unified ToolResolver * All entry points now use the same 4-step fallback chain consistently - Issue #3: Fixed cli_backend support across frameworks * Added resolve_cli_backend_config() in cli_backends module for unified YAML/Python handling * Updated Agent wrapper class to accept both string and dict cli_backend configurations * Added framework compatibility validation with clear error messages * YAML and Python entry points now handle cli_backend identically - Added comprehensive regression tests to prevent future regressions Co-authored-by: MervinPraison <MervinPraison@users.noreply.github.com> * fix: address reviewer feedback - test fixes, validation improvements, error handling - Fix broken test imports and patch paths in test_wrapper_layer_regression.py - Replace hasattr(__call__) with callable() per Ruff B004 - Fix CLI backend validation to handle both 'roles' and 'agents' keys - Add input validation for empty strings and malformed dicts in resolve_cli_backend_config - Include class tools in local tools.py resolution - Add per-tool error handling to prevent single tool failures from breaking all tools - Remove unused logger parameter from resolve_cli_backend_config - Fix exception handling in _resolve_yaml_cli_backend to not mask errors Co-authored-by: Mervin Praison <MervinPraison@users.noreply.github.com> --------- Co-authored-by: praisonai-triage-agent[bot] <272766704+praisonai-triage-agent[bot]@users.noreply.github.com> Co-authored-by: MervinPraison <MervinPraison@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Github actions fix