From 532f128a49eae302bed779cb99f162efcea2612e Mon Sep 17 00:00:00 2001
From: Luis Mendes <luis.mendes@cleverthis.com>
Date: Thu, 12 Mar 2026 14:29:01 +0000
Subject: [PATCH] refactor(test): replace deprecated tempfile.mktemp() with
 tempfile.mkstemp()

Replaced all 28 uses of the deprecated tempfile.mktemp() function with the
atomic tempfile.mkstemp() + os.close(fd) pattern across 16 test infrastructure
and benchmark files.  Added import os where it was not already present.

tempfile.mktemp() has been deprecated since Python 2.3 due to a TOCTOU race
condition: between name generation and file creation, another process could
create a file at the predicted path.  Under 16-worker parallel test execution,
there was also a non-zero risk of path collisions.

Files modified:
- features/environment.py (3 call sites)
- robot/helper_resource_registry_migration.py (3 call sites)
- robot/helper_plan_phase_migration.py (1 call site)
- robot/helper_plan_persistence_e2e.py (1 call site)
- robot/helper_persistence_lifecycle.py (1 call site)
- robot/helper_db_lifecycle_models.py (1 call site)
- robot/helper_automation_profiles.py (1 call site)
- features/steps/skill_discovery_steps.py (2 call sites)
- features/steps/resource_registry_tables_steps.py (2 call sites)
- features/steps/plan_persistence_steps.py (1 call site)
- features/steps/persistence_robot_alignment_steps.py (1 call site)
- features/steps/garbage_collection_cli_steps.py (3 call sites)
- features/steps/decision_recording_steps.py (1 call site)
- features/steps/coverage_boost_steps.py (1 call site)
- features/steps/cli_streaming_steps.py (1 call site)
- features/steps/automation_profiles_guards_steps.py (1 call site)
- benchmarks/persistence_robot_bench.py (4 call sites)

All 10,640 BDD scenarios pass.  No performance regression observed (within
normal run-to-run variance).  Zero remaining uses of tempfile.mktemp() in
the codebase.

ISSUES CLOSED: #730
---
 CHANGELOG.md                                      |  5 +++++
 benchmarks/persistence_robot_bench.py             | 13 +++++++++----
 features/environment.py                           | 15 ++++++++-------
 .../steps/automation_profiles_guards_steps.py     |  4 +++-
 features/steps/cli_streaming_steps.py             |  3 ++-
 features/steps/coverage_boost_steps.py            |  4 +++-
 features/steps/decision_recording_steps.py        |  3 ++-
 features/steps/garbage_collection_cli_steps.py    | 12 +++++++++---
 .../steps/persistence_robot_alignment_steps.py    |  4 +++-
 features/steps/plan_persistence_steps.py          |  4 +++-
 features/steps/resource_registry_tables_steps.py  |  8 ++++++--
 features/steps/skill_discovery_steps.py           |  9 +++++++--
 robot/helper_automation_profiles.py               |  4 +++-
 robot/helper_db_lifecycle_models.py               |  4 +++-
 robot/helper_persistence_lifecycle.py             |  4 +++-
 robot/helper_plan_persistence_e2e.py              |  4 +++-
 robot/helper_plan_phase_migration.py              |  4 +++-
 robot/helper_resource_registry_migration.py       | 10 +++++++---
 18 files changed, 82 insertions(+), 32 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index ef360dd8c..f086752b6 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,11 @@
 
 ## Unreleased
 
+- Replaced all 28 uses of deprecated `tempfile.mktemp()` with atomic
+  `tempfile.mkstemp()` + `os.close(fd)` across 16 test and benchmark files.
+  Eliminates TOCTOU race conditions, prevents potential path collisions under
+  parallel test execution, and removes Python deprecation warnings.  All 10,640
+  BDD scenarios continue to pass under both sequential and parallel modes.  (#730)
 - Added TDD-style failing Behave BDD tests for the session list DI container
   missing `db` provider bug.  Three scenarios exercise `session list`,
   `_get_session_service()`, and `session list --format json` through the real
diff --git a/benchmarks/persistence_robot_bench.py b/benchmarks/persistence_robot_bench.py
index 44a03a641..496dca61f 100644
--- a/benchmarks/persistence_robot_bench.py
+++ b/benchmarks/persistence_robot_bench.py
@@ -7,6 +7,7 @@ close-reopen cycle used in Robot persistence lifecycle tests.
 
 from __future__ import annotations
 
+import os
 import tempfile
 from datetime import UTC, datetime
 from pathlib import Path
@@ -116,7 +117,8 @@ class TimeRobotFixtureSetupSuite:
 
     def time_file_db_init(self) -> None:
         """Measure file-backed SQLite init + schema creation."""
-        tmp: str = tempfile.mktemp(suffix=".db")
+        fd, tmp = tempfile.mkstemp(suffix=".db")
+        os.close(fd)
         try:
             init_database(f"sqlite:///{tmp}")
         finally:
@@ -124,7 +126,8 @@ class TimeRobotFixtureSetupSuite:
 
     def time_seed_action(self) -> None:
         """Measure action seeding into a fresh file DB."""
-        tmp: str = tempfile.mktemp(suffix=".db")
+        fd, tmp = tempfile.mkstemp(suffix=".db")
+        os.close(fd)
         try:
             engine = init_database(f"sqlite:///{tmp}")
             session = Session(bind=engine)
@@ -138,7 +141,8 @@ class TimeRobotFixtureSetupSuite:
 
     def time_seed_plan(self) -> None:
         """Measure plan creation including FK action seeding."""
-        tmp: str = tempfile.mktemp(suffix=".db")
+        fd, tmp = tempfile.mkstemp(suffix=".db")
+        os.close(fd)
         try:
             engine = init_database(f"sqlite:///{tmp}")
             session = Session(bind=engine)
@@ -161,7 +165,8 @@ class TimeRobotRestartCycleSuite:
 
     def setup(self) -> None:
         """Create file DB with action + plan for restart benchmarks."""
-        self._tmp: str = tempfile.mktemp(suffix=".db")
+        fd, self._tmp = tempfile.mkstemp(suffix=".db")
+        os.close(fd)
         engine = init_database(f"sqlite:///{self._tmp}")
         session = Session(bind=engine)
         factory = lambda: session  # noqa: E731
diff --git a/features/environment.py b/features/environment.py
index b65b880c3..d673c955b 100644
--- a/features/environment.py
+++ b/features/environment.py
@@ -44,13 +44,13 @@ def before_all(context):
     # Use per-process unique database paths so parallel test subprocesses
     # (behave-parallel) never contend on the same SQLite file.
     if "CLEVERAGENTS_DATABASE_URL" not in os.environ:
-        os.environ["CLEVERAGENTS_DATABASE_URL"] = (
-            f"sqlite:///{tempfile.mktemp(suffix='.db', prefix='cleveragents_')}"
-        )
+        _fd, _db_path = tempfile.mkstemp(suffix=".db", prefix="cleveragents_")
+        os.close(_fd)
+        os.environ["CLEVERAGENTS_DATABASE_URL"] = f"sqlite:///{_db_path}"
     if "CLEVERAGENTS_TEST_DATABASE_URL" not in os.environ:
-        os.environ["CLEVERAGENTS_TEST_DATABASE_URL"] = (
-            f"sqlite:///{tempfile.mktemp(suffix='.db', prefix='cleveragents_test_')}"
-        )
+        _fd, _db_path = tempfile.mkstemp(suffix=".db", prefix="cleveragents_test_")
+        os.close(_fd)
+        os.environ["CLEVERAGENTS_TEST_DATABASE_URL"] = f"sqlite:///{_db_path}"
     os.environ.setdefault("BEHAVE_TESTING", "true")
 
     # Set up mock AI provider for all tests
@@ -289,7 +289,8 @@ def before_scenario(context, scenario):
             ("CLEVERAGENTS_DATABASE_URL", "cleveragents_"),
             ("CLEVERAGENTS_TEST_DATABASE_URL", "cleveragents_test_"),
         ):
-            db_path = tempfile.mktemp(suffix=".db", prefix=prefix)
+            _fd, db_path = tempfile.mkstemp(suffix=".db", prefix=prefix)
+            os.close(_fd)
             os.environ[env_var] = f"sqlite:///{db_path}"
             context._scenario_db_paths.append(db_path)
 
diff --git a/features/steps/automation_profiles_guards_steps.py b/features/steps/automation_profiles_guards_steps.py
index cd6a94343..e33de5c62 100644
--- a/features/steps/automation_profiles_guards_steps.py
+++ b/features/steps/automation_profiles_guards_steps.py
@@ -1,5 +1,6 @@
 """Step definitions for Automation Profile Guards tests."""
 
+import os
 import tempfile
 from typing import Any
 
@@ -222,7 +223,8 @@ def step_yaml_file_with_guards(context: Context) -> None:
             "require_approval_for_writes": True,
         },
     }
-    tmp_path = tempfile.mktemp(suffix=".yaml")
+    fd, tmp_path = tempfile.mkstemp(suffix=".yaml")
+    os.close(fd)
     with open(tmp_path, "w") as fh:
         yaml.dump(config, fh)
     context.yaml_path = tmp_path
diff --git a/features/steps/cli_streaming_steps.py b/features/steps/cli_streaming_steps.py
index d95e49427..40986a1ca 100644
--- a/features/steps/cli_streaming_steps.py
+++ b/features/steps/cli_streaming_steps.py
@@ -33,7 +33,8 @@ def step_create_temp_dir(context):
 
     # Give this invocation a unique database so repeated calls within the
     # same scenario (e.g. Background + explicit Given) never collide.
-    db_path = tempfile.mktemp(suffix=".db", prefix="cleveragents_")
+    fd, db_path = tempfile.mkstemp(suffix=".db", prefix="cleveragents_")
+    os.close(fd)
     os.environ["CLEVERAGENTS_DATABASE_URL"] = f"sqlite:///{db_path}"
     context._scenario_db_paths = getattr(context, "_scenario_db_paths", [])
     context._scenario_db_paths.append(db_path)
diff --git a/features/steps/coverage_boost_steps.py b/features/steps/coverage_boost_steps.py
index fa0c14ddc..d73e72e9b 100644
--- a/features/steps/coverage_boost_steps.py
+++ b/features/steps/coverage_boost_steps.py
@@ -97,7 +97,9 @@ def step_check_provider_configured(context):
     env_file = Path(".env")
     temp_env = None
     if env_file.exists():
-        temp_env = Path(tempfile.mktemp(suffix=".env"))
+        fd, _tmp_env = tempfile.mkstemp(suffix=".env")
+        os.close(fd)
+        temp_env = Path(_tmp_env)
         shutil.move(str(env_file), str(temp_env))
 
     try:
diff --git a/features/steps/decision_recording_steps.py b/features/steps/decision_recording_steps.py
index da55104c8..b827f9e4a 100644
--- a/features/steps/decision_recording_steps.py
+++ b/features/steps/decision_recording_steps.py
@@ -1038,7 +1038,8 @@ def step_persisted_decision_service(context: Context) -> None:
     """Create a DecisionService backed by a real SQLite database via UnitOfWork."""
     from cleveragents.infrastructure.database.unit_of_work import UnitOfWork
 
-    db_path = tempfile.mktemp(suffix=".db", prefix="dsvc_persisted_")
+    fd, db_path = tempfile.mkstemp(suffix=".db", prefix="dsvc_persisted_")
+    os.close(fd)
     context._dsvc_db_path = db_path
     uow = UnitOfWork(f"sqlite:///{db_path}")
     uow.init_database()
diff --git a/features/steps/garbage_collection_cli_steps.py b/features/steps/garbage_collection_cli_steps.py
index f7b316362..4f52d5df8 100644
--- a/features/steps/garbage_collection_cli_steps.py
+++ b/features/steps/garbage_collection_cli_steps.py
@@ -182,7 +182,9 @@ def step_age_service(context: Context) -> None:
 
 @when("I check the age description for a {hours:d} hour old file")
 def step_age_hours(context: Context, hours: int) -> None:
-    tmp = Path(tempfile.mktemp(prefix="ca-age-test-"))
+    fd, _tmp = tempfile.mkstemp(prefix="ca-age-test-")
+    os.close(fd)
+    tmp = Path(_tmp)
     tmp.write_text("test")
     old_time = time.time() - (hours * 3600)
     os.utime(tmp, (old_time, old_time))
@@ -192,7 +194,9 @@ def step_age_hours(context: Context, hours: int) -> None:
 
 @when("I check the age description for a {minutes:d} minute old file")
 def step_age_minutes(context: Context, minutes: int) -> None:
-    tmp = Path(tempfile.mktemp(prefix="ca-age-test-"))
+    fd, _tmp = tempfile.mkstemp(prefix="ca-age-test-")
+    os.close(fd)
+    tmp = Path(_tmp)
     tmp.write_text("test")
     old_time = time.time() - (minutes * 60)
     os.utime(tmp, (old_time, old_time))
@@ -202,7 +206,9 @@ def step_age_minutes(context: Context, minutes: int) -> None:
 
 @when("I check the age description for a {days:d} day old file")
 def step_age_days(context: Context, days: int) -> None:
-    tmp = Path(tempfile.mktemp(prefix="ca-age-test-"))
+    fd, _tmp = tempfile.mkstemp(prefix="ca-age-test-")
+    os.close(fd)
+    tmp = Path(_tmp)
     tmp.write_text("test")
     old_time = time.time() - (days * 86400)
     os.utime(tmp, (old_time, old_time))
diff --git a/features/steps/persistence_robot_alignment_steps.py b/features/steps/persistence_robot_alignment_steps.py
index 68e65c3d0..f12113c51 100644
--- a/features/steps/persistence_robot_alignment_steps.py
+++ b/features/steps/persistence_robot_alignment_steps.py
@@ -7,6 +7,7 @@ close-reopen cycle exercises real disk persistence.
 
 from __future__ import annotations
 
+import os
 import tempfile
 from datetime import UTC, datetime
 from pathlib import Path
@@ -46,7 +47,8 @@ def _state_from_str(value: str) -> ProcessingState:
 
 @given("a fresh file-backed persistence alignment database")
 def step_fresh_db(context: Context) -> None:
-    tmp_path: str = tempfile.mktemp(suffix=".db")
+    fd, tmp_path = tempfile.mkstemp(suffix=".db")
+    os.close(fd)
     db_url: str = f"sqlite:///{tmp_path}"
     engine: Any = init_database(db_url)
     session: Session = Session(bind=engine)
diff --git a/features/steps/plan_persistence_steps.py b/features/steps/plan_persistence_steps.py
index ab0aaaa51..8aefcd4c1 100644
--- a/features/steps/plan_persistence_steps.py
+++ b/features/steps/plan_persistence_steps.py
@@ -6,6 +6,7 @@ list filters, plan tree links, and cross-restart durability.
 
 from __future__ import annotations
 
+import os
 import tempfile
 from datetime import UTC, datetime
 from pathlib import Path
@@ -101,7 +102,8 @@ def _setup_db(context: Context, *, file_based: bool = False) -> None:
     file.
     """
     if file_based:
-        tmp = tempfile.mktemp(suffix=".db")
+        fd, tmp = tempfile.mkstemp(suffix=".db")
+        os.close(fd)
         db_url = f"sqlite:///{tmp}"
         context._pp_db_path = tmp
     else:
diff --git a/features/steps/resource_registry_tables_steps.py b/features/steps/resource_registry_tables_steps.py
index 42e3a2cf7..8d2ff5f7f 100644
--- a/features/steps/resource_registry_tables_steps.py
+++ b/features/steps/resource_registry_tables_steps.py
@@ -9,6 +9,7 @@ resource_edges).  Uses an in-memory SQLite database with schema created via
 from __future__ import annotations
 
 import json
+import os
 from datetime import UTC, datetime
 from typing import Any
 
@@ -944,7 +945,8 @@ def step_run_init_database(context: Any) -> None:
 
     from cleveragents.infrastructure.database.models import init_database
 
-    tmp = tempfile.mktemp(suffix=".db")
+    fd, tmp = tempfile.mkstemp(suffix=".db")
+    os.close(fd)
     db_url = f"sqlite:///{tmp}"
     engine = init_database(db_url)
     context.rr_migrated_engine = engine
@@ -954,11 +956,13 @@ def step_run_init_database(context: Any) -> None:
 @when("I run init_database twice on the same database file")
 def step_run_init_database_twice(context: Any) -> None:
     """Run init_database twice on the same file to verify idempotency."""
+    import os
     import tempfile
 
     from cleveragents.infrastructure.database.models import init_database
 
-    tmp = tempfile.mktemp(suffix=".db")
+    fd, tmp = tempfile.mkstemp(suffix=".db")
+    os.close(fd)
     db_url = f"sqlite:///{tmp}"
     engine1 = init_database(db_url)
     engine1.dispose()
diff --git a/features/steps/skill_discovery_steps.py b/features/steps/skill_discovery_steps.py
index d11a2dbaa..27b6c80ee 100644
--- a/features/steps/skill_discovery_steps.py
+++ b/features/steps/skill_discovery_steps.py
@@ -2,6 +2,7 @@
 
 from __future__ import annotations
 
+import os
 import shutil
 import tempfile
 from pathlib import Path
@@ -437,7 +438,9 @@ def step_registration_returns(
 
 @when("I scan a non-directory path for agent skills")
 def step_scan_non_dir(context: Context) -> None:
-    tmpfile = Path(tempfile.mktemp(suffix=".txt"))
+    fd, _tmp = tempfile.mkstemp(suffix=".txt")
+    os.close(fd)
+    tmpfile = Path(_tmp)
     tmpfile.write_text("not a directory", encoding="utf-8")
     context.add_cleanup(lambda: tmpfile.unlink(missing_ok=True))
     context.discovered = scan_directory(tmpfile)
@@ -445,7 +448,9 @@ def step_scan_non_dir(context: Context) -> None:
 
 @when("I run discovery on a path that is a file")
 def step_discover_file_path(context: Context) -> None:
-    tmpfile = Path(tempfile.mktemp(suffix=".txt"))
+    fd, _tmp = tempfile.mkstemp(suffix=".txt")
+    os.close(fd)
+    tmpfile = Path(_tmp)
     tmpfile.write_text("not a directory", encoding="utf-8")
     context.add_cleanup(lambda: tmpfile.unlink(missing_ok=True))
     context.discovery_result = discover_agent_skills([tmpfile])
diff --git a/robot/helper_automation_profiles.py b/robot/helper_automation_profiles.py
index a4f3cebf0..e3d8de035 100644
--- a/robot/helper_automation_profiles.py
+++ b/robot/helper_automation_profiles.py
@@ -2,6 +2,7 @@
 
 from __future__ import annotations
 
+import os
 import sys
 import tempfile
 from pathlib import Path
@@ -87,7 +88,8 @@ def _test_from_yaml() -> None:
         "name": "test/yaml",
         "guards": {"max_tool_calls_per_step": 5},
     }
-    tmp_path = tempfile.mktemp(suffix=".yaml")
+    fd, tmp_path = tempfile.mkstemp(suffix=".yaml")
+    os.close(fd)
     with open(tmp_path, "w") as fh:
         yaml.dump(cfg, fh)
     p = AutomationProfile.from_yaml(tmp_path)
diff --git a/robot/helper_db_lifecycle_models.py b/robot/helper_db_lifecycle_models.py
index 8d7211382..e8ec56f70 100644
--- a/robot/helper_db_lifecycle_models.py
+++ b/robot/helper_db_lifecycle_models.py
@@ -8,6 +8,7 @@ Covers the integration between:
 
 from __future__ import annotations
 
+import os
 import sys
 import tempfile
 from datetime import UTC, datetime
@@ -34,7 +35,8 @@ from cleveragents.infrastructure.database.models import (
 
 def _create_temp_db() -> tuple[Session, str]:
     """Create a temporary SQLite database for testing."""
-    tmp = tempfile.mktemp(suffix=".db")
+    fd, tmp = tempfile.mkstemp(suffix=".db")
+    os.close(fd)
     db_url = f"sqlite:///{tmp}"
     engine = init_database(db_url)
     session = Session(bind=engine)
diff --git a/robot/helper_persistence_lifecycle.py b/robot/helper_persistence_lifecycle.py
index 792d8ef3a..5d30c0f7d 100644
--- a/robot/helper_persistence_lifecycle.py
+++ b/robot/helper_persistence_lifecycle.py
@@ -19,6 +19,7 @@ Subcommands:
 
 from __future__ import annotations
 
+import os
 import sys
 import tempfile
 from datetime import UTC, datetime
@@ -60,7 +61,8 @@ from cleveragents.infrastructure.database.repositories import (  # noqa: E402
 
 def _temp_db() -> tuple[str, Session]:
     """Create a file-backed temp SQLite DB and return (path, session)."""
-    tmp: str = tempfile.mktemp(suffix=".db")
+    fd, tmp = tempfile.mkstemp(suffix=".db")
+    os.close(fd)
     db_url: str = f"sqlite:///{tmp}"
     engine = init_database(db_url)
     session = Session(bind=engine)
diff --git a/robot/helper_plan_persistence_e2e.py b/robot/helper_plan_persistence_e2e.py
index 88c8ecc16..030d29af5 100644
--- a/robot/helper_plan_persistence_e2e.py
+++ b/robot/helper_plan_persistence_e2e.py
@@ -10,6 +10,7 @@ Covers:
 
 from __future__ import annotations
 
+import os
 import sys
 import tempfile
 from datetime import UTC, datetime
@@ -45,7 +46,8 @@ from cleveragents.infrastructure.database.repositories import (
 
 def _create_temp_db() -> tuple[str, Session]:
     """Create a temporary SQLite database and return (path, session)."""
-    tmp = tempfile.mktemp(suffix=".db")
+    fd, tmp = tempfile.mkstemp(suffix=".db")
+    os.close(fd)
     db_url = f"sqlite:///{tmp}"
     engine = init_database(db_url)
     session = Session(bind=engine)
diff --git a/robot/helper_plan_phase_migration.py b/robot/helper_plan_phase_migration.py
index d02464d05..bf0d30240 100644
--- a/robot/helper_plan_phase_migration.py
+++ b/robot/helper_plan_phase_migration.py
@@ -6,6 +6,7 @@ and Apply terminal processing states.
 
 from __future__ import annotations
 
+import os
 import sys
 import tempfile
 from datetime import UTC, datetime
@@ -31,7 +32,8 @@ from cleveragents.infrastructure.database.models import (
 
 def _create_temp_db() -> tuple[Session, str]:
     """Create a temporary SQLite database for testing."""
-    tmp = tempfile.mktemp(suffix=".db")
+    fd, tmp = tempfile.mkstemp(suffix=".db")
+    os.close(fd)
     db_url = f"sqlite:///{tmp}"
     engine = init_database(db_url)
     session = Session(bind=engine)
diff --git a/robot/helper_resource_registry_migration.py b/robot/helper_resource_registry_migration.py
index 7c49475e4..9d45da961 100644
--- a/robot/helper_resource_registry_migration.py
+++ b/robot/helper_resource_registry_migration.py
@@ -13,6 +13,7 @@ calling Robot test can assert on ``stdout``.
 
 from __future__ import annotations
 
+import os
 import sys
 import tempfile
 from pathlib import Path
@@ -24,7 +25,8 @@ from cleveragents.infrastructure.database.models import init_database
 
 def _schema_creates_tables() -> None:
     """Verify that init_database creates resource registry tables."""
-    tmp = tempfile.mktemp(suffix=".db")
+    fd, tmp = tempfile.mkstemp(suffix=".db")
+    os.close(fd)
     db_url = f"sqlite:///{tmp}"
     try:
         engine = init_database(db_url)
@@ -41,7 +43,8 @@ def _schema_creates_tables() -> None:
 
 def _table_has_expected_columns() -> None:
     """Verify core columns exist on each resource registry table."""
-    tmp = tempfile.mktemp(suffix=".db")
+    fd, tmp = tempfile.mkstemp(suffix=".db")
+    os.close(fd)
     db_url = f"sqlite:///{tmp}"
     try:
         engine = init_database(db_url)
@@ -83,7 +86,8 @@ def _table_has_expected_columns() -> None:
 
 def _migration_idempotent() -> None:
     """Verify that calling init_database twice is safe (idempotent)."""
-    tmp = tempfile.mktemp(suffix=".db")
+    fd, tmp = tempfile.mkstemp(suffix=".db")
+    os.close(fd)
     db_url = f"sqlite:///{tmp}"
     try:
         engine1 = init_database(db_url)
-- 
2.52.0