From 3ffa6496c80608eadd1179e7953fbb0b481ab72e Mon Sep 17 00:00:00 2001 From: Lavkesh Dwivedi <9712103+lavkeshdwivedi@users.noreply.github.com> Date: Fri, 19 Jun 2026 19:22:44 -0500 Subject: [PATCH 1/5] fix: guard against None rowcount in workflow bulk-delete methods SQLAlchemy's CursorResult.rowcount can return None for some DBAPI drivers/operations. Five bulk-delete methods used result.rowcount directly in += or as a return value, causing: TypeError: unsupported operand type(s) for +=: 'int' and 'NoneType' Apply the `result.rowcount or 0` pattern already used in sibling methods (delete_by_runs, count_by_runs) across all five affected sites: - sqlalchemy_api_workflow_node_execution_repository.py - delete_expired_executions - delete_executions_by_app - delete_executions_by_ids - sqlalchemy_api_workflow_run_repository.py - delete_runs_by_ids - delete_runs_by_app Fixes #37643 --- .../sqlalchemy_api_workflow_node_execution_repository.py | 6 +++--- api/repositories/sqlalchemy_api_workflow_run_repository.py | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/api/repositories/sqlalchemy_api_workflow_node_execution_repository.py b/api/repositories/sqlalchemy_api_workflow_node_execution_repository.py index 0d8f2c09abad48..04b74dcc9c812f 100644 --- a/api/repositories/sqlalchemy_api_workflow_node_execution_repository.py +++ b/api/repositories/sqlalchemy_api_workflow_node_execution_repository.py @@ -286,7 +286,7 @@ def delete_expired_executions( delete_stmt = delete(WorkflowNodeExecutionModel).where(WorkflowNodeExecutionModel.id.in_(execution_ids)) result = cast(CursorResult, session.execute(delete_stmt)) session.commit() - total_deleted += result.rowcount + total_deleted += result.rowcount or 0 # If we deleted fewer than the batch size, we're done if len(execution_ids) < batch_size: @@ -334,7 +334,7 @@ def delete_executions_by_app( delete_stmt = delete(WorkflowNodeExecutionModel).where(WorkflowNodeExecutionModel.id.in_(execution_ids)) result = cast(CursorResult, session.execute(delete_stmt)) session.commit() - total_deleted += result.rowcount + total_deleted += result.rowcount or 0 # If we deleted fewer than the batch size, we're done if len(execution_ids) < batch_size: @@ -393,7 +393,7 @@ def delete_executions_by_ids( stmt = delete(WorkflowNodeExecutionModel).where(WorkflowNodeExecutionModel.id.in_(execution_ids)) result = cast(CursorResult, session.execute(stmt)) session.commit() - return result.rowcount + return result.rowcount or 0 @override def delete_by_runs(self, session: Session, run_ids: Sequence[str]) -> tuple[int, int]: diff --git a/api/repositories/sqlalchemy_api_workflow_run_repository.py b/api/repositories/sqlalchemy_api_workflow_run_repository.py index b40eb4bdd8a3cf..225848c10d316d 100644 --- a/api/repositories/sqlalchemy_api_workflow_run_repository.py +++ b/api/repositories/sqlalchemy_api_workflow_run_repository.py @@ -320,7 +320,7 @@ def delete_runs_by_ids( result = cast(CursorResult, session.execute(stmt)) session.commit() - deleted_count = result.rowcount + deleted_count = result.rowcount or 0 logger.info("Deleted %s workflow runs by IDs", deleted_count) return deleted_count @@ -357,7 +357,7 @@ def delete_runs_by_app( result = cast(CursorResult, session.execute(delete_stmt)) session.commit() - batch_deleted = result.rowcount + batch_deleted = result.rowcount or 0 total_deleted += batch_deleted logger.info("Deleted batch of %s workflow runs for app %s", batch_deleted, app_id) From d3da1464666c7dadc9a7535a4289e85bea1bad7b Mon Sep 17 00:00:00 2001 From: Lavkesh Dwivedi <9712103+lavkeshdwivedi@users.noreply.github.com> Date: Fri, 19 Jun 2026 19:22:49 -0500 Subject: [PATCH 2/5] test: add unit tests for rowcount=None in workflow bulk-delete methods Covers all five affected methods to guard against regression: - delete_expired_executions, delete_executions_by_app, delete_executions_by_ids (node execution repository) - delete_runs_by_ids, delete_runs_by_app (run repository) Each test mocks the DBAPI session to return rowcount=None and asserts the method returns 0 instead of raising TypeError. --- ..._api_workflow_node_execution_repository.py | 47 +++++++++++++++++++ ..._sqlalchemy_api_workflow_run_repository.py | 33 ++++++++++++- 2 files changed, 79 insertions(+), 1 deletion(-) create mode 100644 api/tests/unit_tests/repositories/test_sqlalchemy_api_workflow_node_execution_repository.py diff --git a/api/tests/unit_tests/repositories/test_sqlalchemy_api_workflow_node_execution_repository.py b/api/tests/unit_tests/repositories/test_sqlalchemy_api_workflow_node_execution_repository.py new file mode 100644 index 00000000000000..f270d0d7ca34c9 --- /dev/null +++ b/api/tests/unit_tests/repositories/test_sqlalchemy_api_workflow_node_execution_repository.py @@ -0,0 +1,47 @@ +from __future__ import annotations + +from datetime import UTC, datetime +from unittest.mock import MagicMock + +from repositories.sqlalchemy_api_workflow_node_execution_repository import ( + DifyAPISQLAlchemyWorkflowNodeExecutionRepository, +) + + +def _make_repo() -> tuple[DifyAPISQLAlchemyWorkflowNodeExecutionRepository, MagicMock]: + session = MagicMock() + session_maker = MagicMock() + session_maker.return_value.__enter__.return_value = session + return DifyAPISQLAlchemyWorkflowNodeExecutionRepository(session_maker), session + + +def test_delete_expired_executions_rowcount_none_returns_zero() -> None: + repo, session = _make_repo() + # select returns one ID (< default batch_size=1000), loop exits after first batch + select_result = MagicMock() + select_result.scalars.return_value.all.return_value = ["exec-1"] + delete_result = MagicMock() + delete_result.rowcount = None + session.execute.side_effect = [select_result, delete_result] + + assert repo.delete_expired_executions("tenant-1", datetime(2024, 1, 1, tzinfo=UTC)) == 0 + + +def test_delete_executions_by_app_rowcount_none_returns_zero() -> None: + repo, session = _make_repo() + select_result = MagicMock() + select_result.scalars.return_value.all.return_value = ["exec-1"] + delete_result = MagicMock() + delete_result.rowcount = None + session.execute.side_effect = [select_result, delete_result] + + assert repo.delete_executions_by_app("tenant-1", "app-1") == 0 + + +def test_delete_executions_by_ids_rowcount_none_returns_zero() -> None: + repo, session = _make_repo() + delete_result = MagicMock() + delete_result.rowcount = None + session.execute.return_value = delete_result + + assert repo.delete_executions_by_ids(["exec-1", "exec-2"]) == 0 diff --git a/api/tests/unit_tests/repositories/test_sqlalchemy_api_workflow_run_repository.py b/api/tests/unit_tests/repositories/test_sqlalchemy_api_workflow_run_repository.py index 8f27d0938d0b14..d04e5966d7b9b0 100644 --- a/api/tests/unit_tests/repositories/test_sqlalchemy_api_workflow_run_repository.py +++ b/api/tests/unit_tests/repositories/test_sqlalchemy_api_workflow_run_repository.py @@ -2,11 +2,15 @@ from datetime import UTC, datetime from types import SimpleNamespace +from unittest.mock import MagicMock from graphon.nodes.human_input.entities import FormDefinition, ParagraphInputConfig, UserActionConfig from graphon.nodes.human_input.enums import FormInputType from models.human_input import RecipientType -from repositories.sqlalchemy_api_workflow_run_repository import _build_human_input_required_reason +from repositories.sqlalchemy_api_workflow_run_repository import ( + DifyAPISQLAlchemyWorkflowRunRepository, + _build_human_input_required_reason, +) def _build_form_model() -> SimpleNamespace: @@ -62,3 +66,30 @@ def test_build_human_input_required_reason_falls_back_to_console_token() -> None assert reason.node_id == "node-1" assert reason.actions[0].id == "approve" assert not hasattr(reason, "form_token") + + +def _make_run_repo() -> tuple[DifyAPISQLAlchemyWorkflowRunRepository, MagicMock]: + session = MagicMock() + session_maker = MagicMock() + session_maker.return_value.__enter__.return_value = session + return DifyAPISQLAlchemyWorkflowRunRepository(session_maker), session + + +def test_delete_runs_by_ids_rowcount_none_returns_zero() -> None: + repo, session = _make_run_repo() + delete_result = MagicMock() + delete_result.rowcount = None + session.execute.return_value = delete_result + + assert repo.delete_runs_by_ids(["run-1", "run-2"]) == 0 + + +def test_delete_runs_by_app_rowcount_none_returns_zero() -> None: + repo, session = _make_run_repo() + # select returns one ID (< default batch_size), loop exits after first batch + session.scalars.return_value.all.return_value = ["run-1"] + delete_result = MagicMock() + delete_result.rowcount = None + session.execute.return_value = delete_result + + assert repo.delete_runs_by_app("tenant-1", "app-1") == 0 From 276b96619a231f1dbdf89faf7c5e652ae76a5466 Mon Sep 17 00:00:00 2001 From: Lavkesh Dwivedi <9712103+lavkeshdwivedi@users.noreply.github.com> Date: Fri, 19 Jun 2026 19:28:58 -0500 Subject: [PATCH 3/5] refactor(test): replace SimpleNamespace with real model instances in test_snippet.py Fixes the pyrefly type errors reported in #37604. SimpleNamespace objects passed where Workflow, Account, and Tag are expected do not satisfy the type checker. Replace with real ORM instances so pyrefly can verify the fields being accessed actually exist on the model. Also replace SimpleNamespace session stubs with unittest.mock.Mock, which is the standard pattern in the rest of the test suite. Refs #37604 --- api/tests/unit_tests/models/test_snippet.py | 25 +++++++++++++-------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/api/tests/unit_tests/models/test_snippet.py b/api/tests/unit_tests/models/test_snippet.py index 17f7cb3c9d42bb..072a588c6870dd 100644 --- a/api/tests/unit_tests/models/test_snippet.py +++ b/api/tests/unit_tests/models/test_snippet.py @@ -1,10 +1,11 @@ import json -from types import SimpleNamespace from unittest.mock import Mock import pytest +from models.model import Account, Tag from models.snippet import CustomizedSnippet +from models.workflow import Workflow def test_graph_dict_returns_empty_without_workflow_id() -> None: @@ -14,8 +15,8 @@ def test_graph_dict_returns_empty_without_workflow_id() -> None: def test_graph_dict_loads_published_workflow_graph(monkeypatch: pytest.MonkeyPatch) -> None: - workflow = SimpleNamespace(graph=json.dumps({"nodes": [{"id": "llm-1"}], "edges": []})) - session = SimpleNamespace(get=Mock(return_value=workflow)) + workflow = Workflow(graph=json.dumps({"nodes": [{"id": "llm-1"}], "edges": []})) + session = Mock(get=Mock(return_value=workflow)) monkeypatch.setattr("models.snippet.db.session", session) snippet = CustomizedSnippet(workflow_id="workflow-1") @@ -24,7 +25,7 @@ def test_graph_dict_loads_published_workflow_graph(monkeypatch: pytest.MonkeyPat def test_graph_dict_returns_empty_when_workflow_missing(monkeypatch: pytest.MonkeyPatch) -> None: - session = SimpleNamespace(get=Mock(return_value=None)) + session = Mock(get=Mock(return_value=None)) monkeypatch.setattr("models.snippet.db.session", session) snippet = CustomizedSnippet(workflow_id="missing-workflow") @@ -39,8 +40,10 @@ def test_input_fields_list_parses_json_or_returns_empty() -> None: def test_tags_returns_query_results_or_empty(monkeypatch: pytest.MonkeyPatch) -> None: - tags = [SimpleNamespace(id="tag-1")] - session = SimpleNamespace(scalars=Mock(return_value=SimpleNamespace(all=Mock(return_value=tags)))) + tag = Tag() + tag.id = "tag-1" + tags = [tag] + session = Mock(scalars=Mock(return_value=Mock(all=Mock(return_value=tags)))) monkeypatch.setattr("models.snippet.db.session", session) snippet = CustomizedSnippet(id="snippet-1", tenant_id="tenant-1") @@ -51,9 +54,13 @@ def test_tags_returns_query_results_or_empty(monkeypatch: pytest.MonkeyPatch) -> def test_account_properties_and_author_name(monkeypatch: pytest.MonkeyPatch) -> None: - account = SimpleNamespace(id="account-1", name="Ada") - updated_account = SimpleNamespace(id="account-2", name="Grace") - session = SimpleNamespace( + account = Account() + account.id = "account-1" + account.name = "Ada" + updated_account = Account() + updated_account.id = "account-2" + updated_account.name = "Grace" + session = Mock( get=Mock(side_effect=lambda _model, account_id: account if account_id == "account-1" else updated_account) ) monkeypatch.setattr("models.snippet.db.session", session) From da2b0c14825cc0133d34952807e2712e7aa8d392 Mon Sep 17 00:00:00 2001 From: Lavkesh Dwivedi Date: Sun, 21 Jun 2026 09:21:39 -0500 Subject: [PATCH 4/5] fix(test): add _ConcreteNodeRepo stub to satisfy pyrefly bad-instantiation --- ..._api_workflow_node_execution_repository.py | 22 +++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/api/tests/unit_tests/repositories/test_sqlalchemy_api_workflow_node_execution_repository.py b/api/tests/unit_tests/repositories/test_sqlalchemy_api_workflow_node_execution_repository.py index f270d0d7ca34c9..e24236df06ecb6 100644 --- a/api/tests/unit_tests/repositories/test_sqlalchemy_api_workflow_node_execution_repository.py +++ b/api/tests/unit_tests/repositories/test_sqlalchemy_api_workflow_node_execution_repository.py @@ -1,18 +1,36 @@ from __future__ import annotations +from collections.abc import Sequence from datetime import UTC, datetime from unittest.mock import MagicMock +from core.repositories.factory import OrderConfig +from graphon.entities import WorkflowNodeExecution from repositories.sqlalchemy_api_workflow_node_execution_repository import ( DifyAPISQLAlchemyWorkflowNodeExecutionRepository, ) -def _make_repo() -> tuple[DifyAPISQLAlchemyWorkflowNodeExecutionRepository, MagicMock]: +class _ConcreteNodeRepo(DifyAPISQLAlchemyWorkflowNodeExecutionRepository): + def save(self, execution: WorkflowNodeExecution) -> None: + pass + + def save_execution_data(self, execution: WorkflowNodeExecution) -> None: + pass + + def get_by_workflow_execution( + self, + workflow_execution_id: str, + order_config: OrderConfig | None = None, + ) -> Sequence[WorkflowNodeExecution]: + return [] + + +def _make_repo() -> tuple[_ConcreteNodeRepo, MagicMock]: session = MagicMock() session_maker = MagicMock() session_maker.return_value.__enter__.return_value = session - return DifyAPISQLAlchemyWorkflowNodeExecutionRepository(session_maker), session + return _ConcreteNodeRepo(session_maker), session def test_delete_expired_executions_rowcount_none_returns_zero() -> None: From 6ef1afebe05031efdaa286206ecac68c8a13ea29 Mon Sep 17 00:00:00 2001 From: Lavkesh Dwivedi Date: Sun, 21 Jun 2026 09:21:41 -0500 Subject: [PATCH 5/5] fix(test): add _ConcreteRunRepo stub to satisfy pyrefly bad-instantiation --- .../test_sqlalchemy_api_workflow_run_repository.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/api/tests/unit_tests/repositories/test_sqlalchemy_api_workflow_run_repository.py b/api/tests/unit_tests/repositories/test_sqlalchemy_api_workflow_run_repository.py index d04e5966d7b9b0..212a63bf715555 100644 --- a/api/tests/unit_tests/repositories/test_sqlalchemy_api_workflow_run_repository.py +++ b/api/tests/unit_tests/repositories/test_sqlalchemy_api_workflow_run_repository.py @@ -4,6 +4,7 @@ from types import SimpleNamespace from unittest.mock import MagicMock +from graphon.entities import WorkflowExecution from graphon.nodes.human_input.entities import FormDefinition, ParagraphInputConfig, UserActionConfig from graphon.nodes.human_input.enums import FormInputType from models.human_input import RecipientType @@ -13,6 +14,11 @@ ) +class _ConcreteRunRepo(DifyAPISQLAlchemyWorkflowRunRepository): + def save(self, execution: WorkflowExecution) -> None: + pass + + def _build_form_model() -> SimpleNamespace: expiration_time = datetime(2024, 1, 1, tzinfo=UTC) definition = FormDefinition( @@ -68,11 +74,11 @@ def test_build_human_input_required_reason_falls_back_to_console_token() -> None assert not hasattr(reason, "form_token") -def _make_run_repo() -> tuple[DifyAPISQLAlchemyWorkflowRunRepository, MagicMock]: +def _make_run_repo() -> tuple[_ConcreteRunRepo, MagicMock]: session = MagicMock() session_maker = MagicMock() session_maker.return_value.__enter__.return_value = session - return DifyAPISQLAlchemyWorkflowRunRepository(session_maker), session + return _ConcreteRunRepo(session_maker), session def test_delete_runs_by_ids_rowcount_none_returns_zero() -> None: