Nh/hitl conditional interrupts#1
Conversation
Policy Check Failed✗ 3/3 policy checks failed: • Need 2 more approval(s) (0/2) — comment LGTM or approve via review To merge this PR:
|
PR SummaryWhat Changed
Key Changes by AreaHuman-in-the-Loop Middleware: Added conditional interrupt logic with Testing: Added 6 unit tests covering true/false predicates, mixed tool calls, Files Changed
Review Focus Areas
ArchitectureDesign Decisions
Risks
Merge StatusMERGEABLE — PR Score 66/100, above threshold (50). All gates passed. |
Greptile SummaryThis PR adds a conditional-interrupt feature to
Confidence Score: 3/5Safe to merge after addressing the async-predicate silent-failure in _should_interrupt; all other changes are well-tested and structurally sound. The new _should_interrupt helper calls interrupt_when(tool_call, tool_runtime) and returns the result directly. If a caller passes an async def as the predicate, the call returns an unawaited coroutine object which is always truthy, so every such tool call interrupts regardless of predicate logic, with no error and only a background RuntimeWarning. The rest of the implementation is well-structured and the 8 new tests give good coverage of the synchronous cases. libs/langchain_v1/langchain/agents/middleware/human_in_the_loop.py — specifically the return statement at the end of _should_interrupt. Important Files Changed
Reviews (1): Last reviewed commit: "test(langchain): assert `ToolRuntime.con..." | Re-trigger Greptile |
| interrupt_when = config.get("interrupt_when") | ||
| if interrupt_when is None: | ||
| return True | ||
| tool_runtime: ToolRuntime[ContextT, Any] = ToolRuntime( | ||
| state=state, | ||
| context=runtime.context, | ||
| config={}, | ||
| stream_writer=runtime.stream_writer, | ||
| tool_call_id=tool_call["id"], | ||
| store=runtime.store, | ||
| tools=[], | ||
| execution_info=None, | ||
| server_info=None, | ||
| ) | ||
| return interrupt_when(tool_call, tool_runtime) |
There was a problem hiding this comment.
Async predicate silently bypasses user logic
If a user passes an async def function as interrupt_when, calling interrupt_when(tool_call, tool_runtime) returns a coroutine object rather than a bool. Python coroutine objects are always truthy, so _should_interrupt returns True unconditionally — the interrupt fires regardless of what the predicate was intended to decide, and the coroutine is never awaited (producing only a background RuntimeWarning). Since aafter_model delegates synchronously to after_model, there is no path to correctly handle async predicates. A runtime guard after the call (e.g. asyncio.iscoroutine(result)) would surface this as a clear TypeError rather than silent wrong behavior.
| interrupt_when = config.get("interrupt_when") | |
| if interrupt_when is None: | |
| return True | |
| tool_runtime: ToolRuntime[ContextT, Any] = ToolRuntime( | |
| state=state, | |
| context=runtime.context, | |
| config={}, | |
| stream_writer=runtime.stream_writer, | |
| tool_call_id=tool_call["id"], | |
| store=runtime.store, | |
| tools=[], | |
| execution_info=None, | |
| server_info=None, | |
| ) | |
| return interrupt_when(tool_call, tool_runtime) | |
| import asyncio | |
| result = interrupt_when(tool_call, tool_runtime) | |
| if asyncio.iscoroutine(result): | |
| result.close() | |
| msg = ( | |
| "interrupt_when predicate returned a coroutine. " | |
| "Predicates must be synchronous; pass a plain `def`, not `async def`." | |
| ) | |
| raise TypeError(msg) | |
| return bool(result) |
Security Scan Summary
No critical security issues detected Scan completed in 23.8sSecurity scan powered by Codity.ai |
License Compliance Scan
All licenses are low-risk and compliant Powered by Codity.ai · Docs |
Code Quality Report — test-org-codity/langchain · PR #1Scanned: 2026-05-17 19:17 UTC | Score: 44/100 | Provider: github Executive Summary
Top Findings[CQ-LLM-002]
|
| File | Critical | High | Medium | Low | Total |
|---|---|---|---|---|---|
libs/langchain_v1/langchain/agents/middleware/human_in_the_loop.py |
0 | 0 | 2 | 10 | 12 |
libs/langchain_v1/tests/unit_tests/agents/middleware/implementations/test_human_in_the_loop.py |
0 | 2 | 1 | 8 | 11 |
Recommendations
- Resolve High severity issues, especially error handling gaps and performance bottlenecks.
- Run automated tests after applying fixes to verify no regressions.
Fixes #
Read the full contributing guidelines: https://docs.langchain.com/oss/python/contributing/overview
If you paste a large clearly AI generated description here your PR may be IGNORED or CLOSED!
Thank you for contributing to LangChain! Follow these steps to have your pull request considered as ready for review.
Fixes #xxline at the top is required for external contributions — update the issue number and keep the keyword. This links your PR to the approved issue and auto-closes it on merge.make format,make lintandmake testfrom the root of the package(s) you've modified.Additional guidelines:
uv.lockfiles or add dependencies topyproject.tomlfiles (even optional ones) unless you have explicit permission to do so by a maintainer.Social handles (optional)
Twitter: @
LinkedIn: https://linkedin.com/in/