fix(process_registry): poll() must not mark completion as consumed - #21903
Closed
someaka wants to merge 38 commits into
Closed
fix(process_registry): poll() must not mark completion as consumed#21903someaka wants to merge 38 commits into
someaka wants to merge 38 commits into
Conversation
Previously the return value of memory_manager.on_pre_compress() was discarded (bare statement). Now it is captured and passed to the context compressor as memory_context, which injects it into the summary prompt so memory provider insights survive compaction. Changes: - run_agent.py: capture on_pre_compress() return into memory_context - context_compressor.py: accept memory_context in compress() and _generate_summary(), inject into summary prompt when non-empty - context_engine.py: add memory_context parameter to abstract interface
…aliases - fact_store → holographic_store - fact_feedback → holographic_feedback - Added _TOOL_ALIASES in handle_tool_call for backward compatibility - Updated all user-facing hint text to use new names
- Replace test_second_external_rejected with test_accepts_multiple_external_providers - Add test_rejects_duplicate_provider_name for duplicate name rejection - Add test_remove_provider and related tests for provider deregistration - Add test_remove_provider_cleans_up_tool_routing - Add test_add_provider_after_remove for re-registration lifecycle - All 68 tests in test_memory_provider.py pass
- plugins/memory/__init__.py: fix get_active_memory_providers() to use load_config instead of get_config, update discover_plugin_cli_commands() to iterate all active providers - hermes_cli/memory_setup.py: setup wizard writes to memory.providers list, 'add another?' loop, status shows all providers, _get_configured_providers() / _set_configured_providers() helpers, remove dead _curses_multiselect() code - hermes_cli/main.py: update parser description, memory off clears both provider and providers config keys
All docstrings and comments updated to reflect that multiple memory providers can now be active simultaneously via memory.providers list. - agent/memory_manager.py: module docstring - agent/memory_provider.py: module docstring - hermes_cli/config.py: inline config comment
Doctor now reads memory.providers list (with fallback to memory.provider string) and runs health checks on each active provider individually. Previously only checked a single provider.
Tests now monkeypatch get_active_memory_providers() (returns list) instead of the removed _get_active_memory_provider() (returned string). All 4 discovery tests pass with the new function signature.
… on failure (NousResearch#9948) If get_tool_schemas() raises, the provider must NOT be registered. Previously the bare call would propagate the exception up to the caller. Now it logs an error and returns early, keeping the manager state clean.
- dump.py: support providers list in _memory_provider() - plugins_cmd.py: read/write both providers list and legacy provider - honcho/cli.py: write to providers list alongside legacy provider key - plugins/memory/__init__.py: remove dead _get_active_memory_provider(), remove honcho_command fallback in discover_plugin_cli_commands() - context_compressor.py, memory_provider.py: update docstrings for multi-provider language
- test_schema_loading_failure_prevents_registration: verifies NousResearch#9948 fix - test_namespace_prefix_warning_on_mismatch: verifies naming convention warning - test_tool_budget_warning_above_threshold: verifies 20+ tool count warning - Update plugin CLI registration tests for multi-provider API
- tests/hermes_cli/test_memory_setup.py: tests for _get_configured_providers and _set_configured_providers (list vs legacy format, precedence) - tests/plugins/memory/test_get_active_memory_providers.py: direct unit tests for config-reading logic via monkeypatched load_config
Fixes 7 test files and mcp_tool.py compat issues discovered after rebase onto upstream/main (13a7cbc): - tools/mcp_tool.py: add _CompatType fallback classes - test_browser_chromium_check.py: add shutil.which mocks - test_delegate.py: update 4 assertions - test_credential_pool_env_fallback.py: fix expectations - test_daytona_environment.py: update command wrapper check - test_vercel_sandbox_environment.py: update command wrapper check - test_dockerfile_pid1_reaping.py: update TUI path assertions All tools suite tests now pass.
Function was replaced by get_active_memory_providers() and had zero callers across the entire codebase. Also removes now-unused cfg_get import.
Adds test_remove_provider_shutdown_exception_still_removes to verify that when a provider's shutdown() raises an exception, the provider is still deregistered and its tools are cleaned up from routing.
Adds threading.RLock to MemoryManager to protect _providers and _tool_to_provider state during concurrent add/remove/tool-routing operations. Four new tests verify: - Concurrent add_provider with same name (no duplicates) - Concurrent add_provider with different names (all succeed) - Concurrent add and remove (state remains consistent) - Concurrent tool routing while adding providers (no crashes) 76 tests pass.
Adds pytest.importorskip('acp') to test files that import from acp.schema.
This prevents ImportError collection failures when the optional ACP
dependency is not installed. Tests gracefully skip instead of erroring.
Adds 5 performance tests with time budgets to prevent regressions: - single provider add: <1ms - 10 providers add: <5ms - get_all_tool_schemas from 10 providers: <1ms - remove provider with 50 tools: <1ms - 20 concurrent adds: <50ms All tests pass comfortably within bounds.
_get_current_memory_provider() now joins all providers from the list instead of returning only providers[0]. _save_memory_provider() appends to the existing list instead of nuking it. Also fix test_concurrent_interrupt.py for upstream changes: _tool_guardrails, _append_guardrail_observation, and new kwargs to _invoke_tool (messages=, pre_tool_block_checked=).
…dy active When 'hermes memory setup' is run with existing active providers (e.g. mnemosyne + holographic), the wizard now asks whether to: - Add the new provider alongside existing ones - Replace all existing providers with the new one Previously it silently appended, causing the legacy memory.provider field to drift from the providers list (the status display only read the legacy field, reporting a single provider while two were active). Fixes both interactive cmd_setup() and targeted cmd_setup_provider() entries, including the post_setup delegation path used by hindsight.
- Add _remove_memory_provider(name) to plugins_cmd.py
- Fix _save_memory_provider() legacy key bug (was last-added, now first)
- Replace single-select radio with curses_checklist in _configure_memory_provider()
- Add 'hermes memory remove <provider>' subcommand
- Add DELETE /api/dashboard/plugin-providers/{name} endpoint
- Add memory_providers list field to PUT body and GET response
- Remove 'Add another?' prompt from setup wizard (two locations)
- Add tests: test_plugins_cmd_providers.py (7 tests), update test_memory_setup.py (20 tests)
When active providers exist, the setup picker now includes a 'Remove a provider...' entry. Selecting it shows a checklist of active providers — uncheck the ones to remove, ENTER to confirm.
Add PEP 735 dev dependency group containing fastapi, starlette, uvicorn, mcp, faster-whisper, and pytest-asyncio for development. Updates uv.lock to include the new dev dependencies and refreshes the exclude-newer timestamp.
…multi-provider memory release
…not just when idle Moved the process_registry completion_queue drain from the _agent_running==False idle branch into the top of process_loop() so notifications are delivered immediately regardless of agent state.
PR NousResearch#8228 added _completion_consumed tracking to suppress duplicate completion notifications. The fix correctly applied to wait() and read_log() (both are agent-facing read operations that consume the result), but incorrectly extended to poll(), which is a read-only status query. When poll() marks _completion_consumed, the gateway watcher breaks after the consumed check and never retries, permanently suppressing the notify_on_complete notification. Remove the _completion_consumed.add(session_id) call from poll(). Only wait() and read_log() should mark consumed. Fixes NousResearch#10156
alt-glitch
added
type/bug
Collaborator
Author
|
Superseded by scoped version fix/poll-completion-consumed-scoped — this PR carried 1317 files of unrelated changes |
This was referenced May 25, 2026
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
3 participants
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.
Fix for #10156
PR #8228 added _completion_consumed tracking to suppress duplicate completion notifications. The fix correctly applied to wait() and read_log() (both are agent-facing read operations that consume the result), but incorrectly extended to poll(), which is a read-only status query.
When poll() marks _completion_consumed, the gateway watcher breaks after the consumed check and never retries, permanently suppressing the notify_on_complete notification.
Changes
_completion_consumed.add(session_id)frompoll()intools/process_registry.pytest_poll_marks_completion_consumed→test_poll_does_not_mark_completion_consumedtest_poll_then_wait_marks_consumed,test_poll_then_read_log_marks_consumedFixes #10156