mirror of
https://github.com/Significant-Gravitas/AutoGPT.git
synced 2026-04-08 03:00:28 -04:00
refactor(copilot): make dry_run a mandatory parameter in tool signatures
Remove `= False` default from dry_run in _execute, execute_block, prepare_block_for_execution, RunAgentInput, and _run_agent so callers must always pass it explicitly. This prevents ambiguity about which execution mode is active. - Add dry_run to "required" in RunBlockTool and RunAgentTool JSON schemas - Make RunBlockTool._execute params keyword-only (after *) for pyright compat - Update all test call sites to pass dry_run=True or dry_run=False explicitly - Leave ChatSessionMetadata.dry_run default intact (DB model)
This commit is contained in:
@@ -81,7 +81,7 @@ async def execute_block(
|
||||
node_exec_id: str,
|
||||
matched_credentials: dict[str, CredentialsMetaInput],
|
||||
sensitive_action_safe_mode: bool = False,
|
||||
dry_run: bool = False,
|
||||
dry_run: bool,
|
||||
) -> ToolResponseBase:
|
||||
"""Execute a block with full context setup, credential injection, and error handling.
|
||||
|
||||
@@ -337,7 +337,7 @@ async def prepare_block_for_execution(
|
||||
user_id: str,
|
||||
session: ChatSession,
|
||||
session_id: str,
|
||||
dry_run: bool = False,
|
||||
dry_run: bool,
|
||||
) -> "BlockPreparation | ToolResponseBase":
|
||||
"""Validate and prepare a block for execution.
|
||||
|
||||
|
||||
@@ -102,6 +102,7 @@ class TestExecuteBlockCreditCharging:
|
||||
session_id=_SESSION,
|
||||
node_exec_id="exec-1",
|
||||
matched_credentials={},
|
||||
dry_run=False,
|
||||
)
|
||||
|
||||
assert isinstance(result, BlockOutputResponse)
|
||||
@@ -132,6 +133,7 @@ class TestExecuteBlockCreditCharging:
|
||||
session_id=_SESSION,
|
||||
node_exec_id="exec-1",
|
||||
matched_credentials={},
|
||||
dry_run=False,
|
||||
)
|
||||
|
||||
assert isinstance(result, ErrorResponse)
|
||||
@@ -158,6 +160,7 @@ class TestExecuteBlockCreditCharging:
|
||||
session_id=_SESSION,
|
||||
node_exec_id="exec-1",
|
||||
matched_credentials={},
|
||||
dry_run=False,
|
||||
)
|
||||
|
||||
assert isinstance(result, BlockOutputResponse)
|
||||
@@ -194,6 +197,7 @@ class TestExecuteBlockCreditCharging:
|
||||
session_id=_SESSION,
|
||||
node_exec_id="exec-1",
|
||||
matched_credentials={},
|
||||
dry_run=False,
|
||||
)
|
||||
|
||||
# Block already executed (with side effects), so output is returned
|
||||
@@ -277,6 +281,7 @@ async def test_coerce_json_string_to_nested_list():
|
||||
session_id=_TEST_SESSION_ID,
|
||||
node_exec_id="exec-1",
|
||||
matched_credentials={},
|
||||
dry_run=False,
|
||||
)
|
||||
|
||||
assert isinstance(response, BlockOutputResponse)
|
||||
@@ -317,6 +322,7 @@ async def test_coerce_json_string_to_list():
|
||||
session_id=_TEST_SESSION_ID,
|
||||
node_exec_id="exec-2",
|
||||
matched_credentials={},
|
||||
dry_run=False,
|
||||
)
|
||||
|
||||
assert isinstance(response, BlockOutputResponse)
|
||||
@@ -349,6 +355,7 @@ async def test_coerce_json_string_to_dict():
|
||||
session_id=_TEST_SESSION_ID,
|
||||
node_exec_id="exec-3",
|
||||
matched_credentials={},
|
||||
dry_run=False,
|
||||
)
|
||||
|
||||
assert isinstance(response, BlockOutputResponse)
|
||||
@@ -382,6 +389,7 @@ async def test_no_coercion_when_type_matches():
|
||||
session_id=_TEST_SESSION_ID,
|
||||
node_exec_id="exec-4",
|
||||
matched_credentials={},
|
||||
dry_run=False,
|
||||
)
|
||||
|
||||
assert isinstance(response, BlockOutputResponse)
|
||||
@@ -415,6 +423,7 @@ async def test_coerce_string_to_int():
|
||||
session_id=_TEST_SESSION_ID,
|
||||
node_exec_id="exec-5",
|
||||
matched_credentials={},
|
||||
dry_run=False,
|
||||
)
|
||||
|
||||
assert isinstance(response, BlockOutputResponse)
|
||||
@@ -448,6 +457,7 @@ async def test_coerce_skips_none_values():
|
||||
session_id=_TEST_SESSION_ID,
|
||||
node_exec_id="exec-6",
|
||||
matched_credentials={},
|
||||
dry_run=False,
|
||||
)
|
||||
|
||||
assert isinstance(response, BlockOutputResponse)
|
||||
@@ -481,6 +491,7 @@ async def test_coerce_union_type_preserves_valid_member():
|
||||
session_id=_TEST_SESSION_ID,
|
||||
node_exec_id="exec-7",
|
||||
matched_credentials={},
|
||||
dry_run=False,
|
||||
)
|
||||
|
||||
assert isinstance(response, BlockOutputResponse)
|
||||
@@ -516,6 +527,7 @@ async def test_coerce_inner_elements_of_generic():
|
||||
session_id=_TEST_SESSION_ID,
|
||||
node_exec_id="exec-8",
|
||||
matched_credentials={},
|
||||
dry_run=False,
|
||||
)
|
||||
|
||||
assert isinstance(response, BlockOutputResponse)
|
||||
@@ -592,6 +604,7 @@ async def test_prepare_block_not_found() -> None:
|
||||
user_id=_PREP_USER,
|
||||
session=_make_prep_session(),
|
||||
session_id=_PREP_SESSION,
|
||||
dry_run=False,
|
||||
)
|
||||
assert isinstance(result, ErrorResponse)
|
||||
assert "not found" in result.message
|
||||
@@ -612,6 +625,7 @@ async def test_prepare_block_disabled() -> None:
|
||||
user_id=_PREP_USER,
|
||||
session=_make_prep_session(),
|
||||
session_id=_PREP_SESSION,
|
||||
dry_run=False,
|
||||
)
|
||||
assert isinstance(result, ErrorResponse)
|
||||
assert "disabled" in result.message
|
||||
@@ -640,6 +654,7 @@ async def test_prepare_block_unrecognized_fields() -> None:
|
||||
user_id=_PREP_USER,
|
||||
session=_make_prep_session(),
|
||||
session_id=_PREP_SESSION,
|
||||
dry_run=False,
|
||||
)
|
||||
assert isinstance(result, InputValidationErrorResponse)
|
||||
assert "unknown_field" in result.unrecognized_fields
|
||||
@@ -669,6 +684,7 @@ async def test_prepare_block_missing_credentials() -> None:
|
||||
user_id=_PREP_USER,
|
||||
session=_make_prep_session(),
|
||||
session_id=_PREP_SESSION,
|
||||
dry_run=False,
|
||||
)
|
||||
assert isinstance(result, SetupRequirementsResponse)
|
||||
|
||||
@@ -698,6 +714,7 @@ async def test_prepare_block_success_returns_preparation() -> None:
|
||||
user_id=_PREP_USER,
|
||||
session=_make_prep_session(),
|
||||
session_id=_PREP_SESSION,
|
||||
dry_run=False,
|
||||
)
|
||||
assert isinstance(result, BlockPreparation)
|
||||
assert result.required_non_credential_keys == {"text"}
|
||||
@@ -802,6 +819,7 @@ async def test_prepare_block_excluded_by_type() -> None:
|
||||
user_id=_PREP_USER,
|
||||
session=_make_prep_session(),
|
||||
session_id=_PREP_SESSION,
|
||||
dry_run=False,
|
||||
)
|
||||
assert isinstance(result, ErrorResponse)
|
||||
assert "cannot be run directly" in result.message
|
||||
@@ -824,6 +842,7 @@ async def test_prepare_block_excluded_by_id() -> None:
|
||||
user_id=_PREP_USER,
|
||||
session=_make_prep_session(),
|
||||
session_id=_PREP_SESSION,
|
||||
dry_run=False,
|
||||
)
|
||||
assert isinstance(result, ErrorResponse)
|
||||
assert "cannot be run directly" in result.message
|
||||
@@ -857,6 +876,7 @@ async def test_prepare_block_file_ref_expansion_error() -> None:
|
||||
user_id=_PREP_USER,
|
||||
session=_make_prep_session(),
|
||||
session_id=_PREP_SESSION,
|
||||
dry_run=False,
|
||||
)
|
||||
assert isinstance(result, ErrorResponse)
|
||||
assert "file reference" in result.message.lower()
|
||||
|
||||
@@ -71,7 +71,7 @@ class RunAgentInput(BaseModel):
|
||||
cron: str = ""
|
||||
timezone: str = "UTC"
|
||||
wait_for_result: int = Field(default=0, ge=0, le=300)
|
||||
dry_run: bool = False
|
||||
dry_run: bool
|
||||
|
||||
@field_validator(
|
||||
"username_agent_slug",
|
||||
@@ -160,7 +160,7 @@ class RunAgentTool(BaseTool):
|
||||
),
|
||||
},
|
||||
},
|
||||
"required": [],
|
||||
"required": ["dry_run"],
|
||||
}
|
||||
|
||||
@property
|
||||
@@ -478,8 +478,8 @@ class RunAgentTool(BaseTool):
|
||||
graph: GraphModel,
|
||||
graph_credentials: dict[str, CredentialsMetaInput],
|
||||
inputs: dict[str, Any],
|
||||
dry_run: bool,
|
||||
wait_for_result: int = 0,
|
||||
dry_run: bool = False,
|
||||
) -> ToolResponseBase:
|
||||
"""Execute an agent immediately, optionally waiting for completion."""
|
||||
session_id = session.session_id
|
||||
|
||||
@@ -54,11 +54,11 @@ class RunBlockTool(BaseTool):
|
||||
"description": (
|
||||
"When true, simulates block execution using an LLM without making any "
|
||||
"real API calls or producing side effects. Useful for testing agent "
|
||||
"wiring and previewing outputs. Default: false."
|
||||
"wiring and previewing outputs."
|
||||
),
|
||||
},
|
||||
},
|
||||
"required": ["block_id", "input_data"],
|
||||
"required": ["block_id", "input_data", "dry_run"],
|
||||
}
|
||||
|
||||
@property
|
||||
@@ -69,9 +69,10 @@ class RunBlockTool(BaseTool):
|
||||
self,
|
||||
user_id: str | None,
|
||||
session: ChatSession,
|
||||
*,
|
||||
block_id: str = "",
|
||||
input_data: dict | None = None,
|
||||
dry_run: bool = False,
|
||||
dry_run: bool,
|
||||
**kwargs,
|
||||
) -> ToolResponseBase:
|
||||
"""Execute a block with the given input data.
|
||||
|
||||
@@ -103,6 +103,7 @@ class TestRunBlockFiltering:
|
||||
session=session,
|
||||
block_id="input-block-id",
|
||||
input_data={},
|
||||
dry_run=False,
|
||||
)
|
||||
|
||||
assert isinstance(response, ErrorResponse)
|
||||
@@ -129,6 +130,7 @@ class TestRunBlockFiltering:
|
||||
session=session,
|
||||
block_id=orchestrator_id,
|
||||
input_data={},
|
||||
dry_run=False,
|
||||
)
|
||||
|
||||
assert isinstance(response, ErrorResponse)
|
||||
@@ -154,6 +156,7 @@ class TestRunBlockFiltering:
|
||||
session=session,
|
||||
block_id=block_id,
|
||||
input_data={},
|
||||
dry_run=False,
|
||||
)
|
||||
finally:
|
||||
_current_permissions.reset(token)
|
||||
@@ -187,6 +190,7 @@ class TestRunBlockFiltering:
|
||||
session=session,
|
||||
block_id=block_id,
|
||||
input_data={},
|
||||
dry_run=False,
|
||||
)
|
||||
finally:
|
||||
_current_permissions.reset(token)
|
||||
@@ -222,6 +226,7 @@ class TestRunBlockFiltering:
|
||||
session=session,
|
||||
block_id="standard-id",
|
||||
input_data={},
|
||||
dry_run=False,
|
||||
)
|
||||
|
||||
# Should NOT be an ErrorResponse about CoPilot exclusion
|
||||
@@ -282,6 +287,7 @@ class TestRunBlockInputValidation:
|
||||
"prompt": "Write a haiku about coding",
|
||||
"LLM_Model": "claude-opus-4-6",
|
||||
},
|
||||
dry_run=False,
|
||||
)
|
||||
|
||||
assert isinstance(response, InputValidationErrorResponse)
|
||||
@@ -327,6 +333,7 @@ class TestRunBlockInputValidation:
|
||||
"system_prompt": "Be helpful",
|
||||
"retries": 5,
|
||||
},
|
||||
dry_run=False,
|
||||
)
|
||||
|
||||
assert isinstance(response, InputValidationErrorResponse)
|
||||
@@ -370,6 +377,7 @@ class TestRunBlockInputValidation:
|
||||
input_data={
|
||||
"LLM_Model": "claude-opus-4-6",
|
||||
},
|
||||
dry_run=False,
|
||||
)
|
||||
|
||||
assert isinstance(response, InputValidationErrorResponse)
|
||||
@@ -424,6 +432,7 @@ class TestRunBlockInputValidation:
|
||||
"prompt": "Write a haiku",
|
||||
"model": "gpt-4o-mini",
|
||||
},
|
||||
dry_run=False,
|
||||
)
|
||||
|
||||
assert isinstance(response, BlockOutputResponse)
|
||||
@@ -463,6 +472,7 @@ class TestRunBlockInputValidation:
|
||||
input_data={
|
||||
"model": "gpt-4o-mini",
|
||||
},
|
||||
dry_run=False,
|
||||
)
|
||||
|
||||
assert isinstance(response, BlockDetailsResponse)
|
||||
@@ -514,6 +524,7 @@ class TestRunBlockSensitiveAction:
|
||||
session=session,
|
||||
block_id="delete-branch-id",
|
||||
input_data=input_data,
|
||||
dry_run=False,
|
||||
)
|
||||
|
||||
assert isinstance(response, ReviewRequiredResponse)
|
||||
@@ -574,6 +585,7 @@ class TestRunBlockSensitiveAction:
|
||||
session=session,
|
||||
block_id="delete-branch-id",
|
||||
input_data=input_data,
|
||||
dry_run=False,
|
||||
)
|
||||
|
||||
assert isinstance(response, BlockOutputResponse)
|
||||
@@ -628,6 +640,7 @@ class TestRunBlockSensitiveAction:
|
||||
session=session,
|
||||
block_id="http-request-id",
|
||||
input_data=input_data,
|
||||
dry_run=False,
|
||||
)
|
||||
|
||||
assert isinstance(response, BlockOutputResponse)
|
||||
|
||||
@@ -307,11 +307,12 @@ async def test_execute_block_real_execution_unchanged():
|
||||
|
||||
|
||||
def test_run_block_tool_dry_run_param():
|
||||
"""RunBlockTool parameters should include 'dry_run'."""
|
||||
"""RunBlockTool parameters should include 'dry_run' as a required field."""
|
||||
tool = RunBlockTool()
|
||||
params = tool.parameters
|
||||
assert "dry_run" in params["properties"]
|
||||
assert params["properties"]["dry_run"]["type"] == "boolean"
|
||||
assert "dry_run" in params["required"]
|
||||
|
||||
|
||||
def test_run_block_tool_dry_run_calls_execute():
|
||||
|
||||
@@ -76,6 +76,7 @@ async def test_run_block_returns_details_when_no_input_provided():
|
||||
session=session,
|
||||
block_id="http-block-id",
|
||||
input_data={}, # Empty input data
|
||||
dry_run=False,
|
||||
)
|
||||
|
||||
# Should return BlockDetailsResponse showing the schema
|
||||
@@ -143,6 +144,7 @@ async def test_run_block_returns_details_when_only_credentials_provided():
|
||||
session=session,
|
||||
block_id="api-block-id",
|
||||
input_data={"credentials": {"some": "cred"}}, # Only credential
|
||||
dry_run=False,
|
||||
)
|
||||
|
||||
# Should return details because no non-credential inputs provided
|
||||
|
||||
@@ -273,8 +273,8 @@ class TestChatSessionDryRun:
|
||||
class TestRunAgentInputDryRunOverride:
|
||||
"""Test that RunAgentInput.dry_run can be mutated by session-level override."""
|
||||
|
||||
def test_default_dry_run_false(self):
|
||||
params = RunAgentInput(username_agent_slug="user/agent")
|
||||
def test_explicit_dry_run_false(self):
|
||||
params = RunAgentInput(username_agent_slug="user/agent", dry_run=False)
|
||||
assert params.dry_run is False
|
||||
|
||||
def test_session_override(self):
|
||||
|
||||
Reference in New Issue
Block a user