-
Notifications
You must be signed in to change notification settings - Fork 110
[DRAFT] Implements hook-based HITL in 2 working modes - 1) triggered by agent (e.g. when operation is destructive) 2) triggered by the pentester #74
Conversation
PR Quality CheckPylint Score: 9.64/10 [PASS] [PASS] All checks passed! |
|
@claude Please review this HITL implementation PR. Focus on:
Be direct and constructive. |
|
Claude finished @westonbrown's task —— View job Code Review: HITL ImplementationConducted comprehensive review of the Human-in-the-Loop implementation across 27 files (3889 additions, 316 deletions). 1. Code Quality & ArchitectureOver-Engineering Concerns
|
4c4ce79 to
6c3361f
Compare
|
@westonbrown @aggr0cr4g did some further changes and test for the agent-triggered HITL panel. On a high-level I can confirm that the it seems to work (to the extent I had the time to test), but I'm still not 100% sure. For instance: once it asked me for feedback for an operation and I rejected it, and then the agent crashed. On another instance I approved and it seemed to work. Haven't really gotten too much into testing with a correction. But bottomline is that it's triggered either a) when a potentially descructive operation is detected (this needs to be further hardened) or b) when agent has low confidence (ATM set it to 90% to get the trigger to kick in ASAP). I think intuitively it works as expected, but needs much more testing to really confirm everything How to test
Still outstanding
|
|
Base branch changed: |
|
Thanks for pushing this — looks promising. Could you:
I'd like to land this as a single PR and goal is to merge by this weekend. I can help test and review once rebased; community help welcome — especially around pause/resume and the agent‑triggered path. |
|
hey @westonbrown - thanks! Yeah, will do an update in the next 1-2 days :) |
|
@westonbrown working on it now but won't have enough time to complete entirely. I should be however able to finish it by Tuesday EOD if that's fine with you. |
…avior - Create test_hitl_feedback_injection.py with 27 test cases - Test feedback injection hook without requiring live LLM - Cover all feedback types: CORRECTION, SUGGESTION, APPROVAL, REJECTION - Verify system prompt modification and feedback clearing - Test sequential feedback cycles and edge cases - Tests are CI-friendly and run in milliseconds Addresses feedback injection testing requirements: - Debug agent response behavior for HITL feedback - Test different feedback message formats - Validate system prompt modification effectiveness - Use mocked agent instead of live Ollama Co-authored-by: Aaron Brown <westonbrown@users.noreply.github.com>
Enhance the agent-triggered HITL intervention panel based on user feedback and testing: - Fix yellow box alignment by using paddingX instead of padding - Update panel title to sentence case for less aggressive tone - Reorder action options: approve, reject, correction - Fix correction feedback input mode activation - Add rejection handling in hook provider to prevent tool execution - Document HITL triggering conditions in system prompt The rejection handler now raises RuntimeError to prevent destructive operations when user clicks reject, while still allowing the agent to receive the rejection feedback at next model invocation.
9302028 to
7a8cdcb
Compare
PR Quality CheckPylint Score: N/A/10 [FAIL] [FAIL] Pylint score must be 9.5 or higher |
PR Quality CheckPylint Score: 9.60/10 [PASS] [PASS] All checks passed! |
PR Quality CheckPylint Score: 9.60/10 [PASS] [PASS] All checks passed! |
Removed duplicate class definitions that were causing isinstance() validation failures. Classes were being imported from modules.config.types but then redefined in manager.py, creating two different class objects with the same name. Changes: - Removed duplicate definitions: ModelProvider, ModelConfig, LLMConfig, EmbeddingConfig, VectorStoreConfig, MemoryLLMConfig, MemoryEmbeddingConfig, MemoryVectorStoreConfig, MemoryConfig, EvaluationConfig, SwarmConfig, get_default_base_dir, SDKConfig, OutputConfig, ServerConfig - Moved HITLConfig to types.py and added to ServerConfig - Added HITLConfig to imports in manager.py - Removed stale MEM0_PROVIDER_MAP duplicate (more complete version in types.py) - Cleaned up merge conflict markers
PR Quality CheckPylint Score: 9.71/10 [PASS] [PASS] All checks passed! |
Remove unconditional hooks list recreation that caused UnboundLocalError when prompt optimization is disabled. The hook is only created when CYBER_ENABLE_PROMPT_OPTIMIZATION is enabled, so we shouldn't reference it unconditionally.
Test mocks were missing required hitl configuration attribute causing AttributeError. Added hitl SimpleNamespace with default disabled config to all affected test fixtures.
create_agent now returns three values (agent, callback_handler, feedback_manager) instead of two. Updated all test assertions to unpack three values correctly.
Adjust test expectations to work with actual snapshot data instead of requiring specific mock data counts. Tests now use real client or have flexible assertions that work with any valid snapshot.
Remove undefined feedback_injected_this_turn variable reference and duplicate get_config_manager import. Add missing Dict type import for type annotations.
Apply automatic code formatting to maintain consistent style across the codebase.
PR Quality CheckPylint Score: 9.72/10 [PASS] [PASS] All checks passed! |
Add missing mocks (get_config_manager, ReactBridgeHandler) that are required for the test to reach the model creation code. Without these mocks, the function fails earlier and never reaches the error handler being tested.
PR Quality CheckPylint Score: 9.72/10 [PASS] [PASS] All checks passed! |
Manual test test_hitl_hook_manual requires Ollama server running locally, which is not available in CI environment. This was causing test failures with connection errors. Changes: - Add 'manual' pytest marker in pyproject.toml - Mark test_feedback_injection_hook with @pytest.mark.manual - Update CI pytest command to skip manual tests with -m 'not manual' - Add documentation on how to run manual tests locally Manual tests can still be run with: pytest -m manual pytest tests/test_hitl_hook_manual.py
PR Quality CheckPylint Score: 9.72/10 [PASS] [PASS] All checks passed! |
The pr-checks.yml workflow was missing the -m 'not manual' flag, causing manual integration tests to run on PRs. This resulted in test failures when Ollama server is not available in CI. This completes the fix started in commit 5bed705 by ensuring both ci.yml and pr-checks.yml workflows skip manual tests.
PR Quality CheckPylint Score: 9.72/10 [PASS] [PASS] All checks passed! |

Summary
Implement Human-in-the-Loop (HITL) feedback system enabling real-time intervention and guidance during agent execution through React Terminal UI.
Architecture & Implementation
Core Components
Backend (Python)
Frontend (React/TypeScript)
NOTE: pause & resume are currently not working yet and need to be implemented and tested extensively
Two Working Modes
1. Agent-Triggered HITL
NOTE: needs further extension and clarification WRT events/ operations that should trigger the event. How to make it configurable
NOTE: should be made configurable, still TBD
2. Pentester-Triggered HITL
Technical Flow
User Input (React) → Stdin Communication → Python Background Thread →
FeedbackManager State → Strands Hook System → System Prompt Injection →
LLM Receives Modified Context → Agent Adjusts Behavior
Implementation Details
Current Status & Testing
Working Features
NOTE: still needs further practical testing and the agent-triggered HITL has not been extensively reviewed
Testing/ Debugging Focus
Next Steps
Priority: Debug agent response behavior