test: address agent runner review comments

This commit is contained in:
huanghuoguoguo
2026-05-17 10:41:02 +08:00
committed by huanghuoguoguo
parent 563131ad52
commit 0cdecbbf36
2 changed files with 13 additions and 30 deletions
+9 -24
View File
@@ -431,11 +431,9 @@ class TestAuthorizationPathDifferentiation:
# Regular plugin call has no run_id # Regular plugin call has no run_id
run_id = None run_id = None
# Authorization check should be skipped when run_id is None # Authorization check should be skipped when run_id is None.
# This is handled in handler.py with: if run_id: ... # This is handled in handler.py with: if run_id: ...
if run_id: assert run_id is None
# This block should NOT execute for regular plugin calls
raise AssertionError('Authorization check should not run for regular plugin calls')
# For regular plugins: # For regular plugins:
# - INVOKE_LLM: unrestricted access to any model # - INVOKE_LLM: unrestricted access to any model
@@ -661,12 +659,7 @@ class TestHandlerActionAuthorization:
run_id = None run_id = None
# In handler.py, authorization check is inside: if run_id: ... # In handler.py, authorization check is inside: if run_id: ...
# So when run_id is None, authorization is skipped # So when run_id is None, authorization is skipped.
if run_id:
# This block should NOT execute
raise AssertionError('Should not execute authorization for no run_id')
# No authorization check - unrestricted access
assert run_id is None assert run_id is None
@pytest.mark.asyncio @pytest.mark.asyncio
@@ -733,9 +726,6 @@ class TestHandlerActionAuthorization:
run_id = None run_id = None
# Authorization check is inside: if run_id: ... # Authorization check is inside: if run_id: ...
if run_id:
raise AssertionError('Should not execute authorization for no run_id')
assert run_id is None assert run_id is None
@pytest.mark.asyncio @pytest.mark.asyncio
@@ -880,10 +870,8 @@ class TestNoRunIdBackwardCompatPath:
run_id = None run_id = None
llm_model_uuid = 'any_model' llm_model_uuid = 'any_model'
# Simulate handler logic # Simulate handler logic: no run_id skips authorization.
if run_id: assert run_id is None
# This should NOT execute
raise AssertionError('Authorization should not run')
# Model can be any UUID (unrestricted) # Model can be any UUID (unrestricted)
assert llm_model_uuid == 'any_model' assert llm_model_uuid == 'any_model'
@@ -895,8 +883,7 @@ class TestNoRunIdBackwardCompatPath:
tool_name = 'any_tool' tool_name = 'any_tool'
# Handler.py line 463: if run_id: ... # Handler.py line 463: if run_id: ...
if run_id: assert run_id is None
raise AssertionError('Authorization should not run')
assert tool_name == 'any_tool' assert tool_name == 'any_tool'
@@ -1982,9 +1969,8 @@ class TestBackwardCompatStorageNoRunId:
# When run_id is None, validation is skipped # When run_id is None, validation is skipped
run_id = None run_id = None
# Simulate handler logic # Simulate handler logic: no run_id skips validation.
if run_id: assert run_id is None
raise AssertionError('Should not execute validation')
# Storage access unrestricted for regular plugins # Storage access unrestricted for regular plugins
assert run_id is None assert run_id is None
@@ -1993,8 +1979,7 @@ class TestBackwardCompatStorageNoRunId:
"""GET_CONFIG_FILE without run_id skips Host-side validation.""" """GET_CONFIG_FILE without run_id skips Host-side validation."""
run_id = None run_id = None
if run_id: assert run_id is None
raise AssertionError('Should not execute validation')
# File access unrestricted for regular plugins # File access unrestricted for regular plugins
assert run_id is None assert run_id is None
@@ -27,8 +27,6 @@ class TestSessionRegistryBasic:
models=[{'model_id': 'model_001', 'model_type': 'chat', 'provider': 'openai'}], models=[{'model_id': 'model_001', 'model_type': 'chat', 'provider': 'openai'}],
tools=[{'tool_name': 'web_search', 'tool_type': 'builtin'}], tools=[{'tool_name': 'web_search', 'tool_type': 'builtin'}],
) )
session = make_session(run_id=run_id, resources=resources)
await registry.register( await registry.register(
run_id=run_id, run_id=run_id,
runner_id='plugin:test/my-runner/default', runner_id='plugin:test/my-runner/default',
@@ -393,10 +391,10 @@ class TestGlobalRegistry:
# In production, calling get_session_registry() multiple times # In production, calling get_session_registry() multiple times
# returns the same instance. We verify this by checking the # returns the same instance. We verify this by checking the
# module-level variable directly. # module-level variable directly.
import langbot.pkg.agent.runner.session_registry as registry_module from langbot.pkg.agent.runner.session_registry import _global_registry
# Check that the global variable exists and is either None or an instance # Check that the global variable exists and is either None or an instance
global_reg = registry_module._global_registry global_reg = _global_registry
if global_reg is None: if global_reg is None:
# First call creates the instance # First call creates the instance
registry1 = get_session_registry() registry1 = get_session_registry()
@@ -453,8 +451,8 @@ class TestThreadSafety:
registry.get('run_1'), registry.get('run_1'),
] ]
results = await asyncio.gather(*tasks) await asyncio.gather(*tasks)
# After both complete, session should be unregistered # After both complete, session should be unregistered
result = await registry.get('run_1') result = await registry.get('run_1')
assert result is None assert result is None