fix(permissions): narrow rm -rf deny globs to their exact forms

The trailing wildcard on the three rm -rf deny entries spanned path separators, so Bash(rm -rf /*) matched every absolute path on the machine rather than the filesystem root, and the ~ and $HOME entries had the same shape. Narrowed to the exact literal forms. These rules match literal command text, so they still stop a typo on rm -rf /, rm -rf ~ or rm -rf $HOME exactly, but they no longer stop a recursive delete aimed at any other path. That reduced cover is deliberate, not an oversight.
build(make): deselect the integration tests, and give them a target (T-55)
2026-08-25 20:31:21 +02:00 · 2026-08-18 20:44:24 +02:00 · 2026-08-18 20:44:08 +02:00 · 2026-08-18 20:44:08 +02:00 · 2026-08-18 15:50:45 +02:00 · 2026-08-18 15:50:29 +02:00
9 changed files with 526 additions and 498 deletions
@@ -58,9 +58,9 @@
      "Bash(psql * TRUNCATE*)",
      "Bash(redis-cli * FLUSHALL*)",
      "Bash(redis-cli * FLUSHDB*)",
-      "Bash(rm -rf $HOME*)",
-      "Bash(rm -rf /*)",
-      "Bash(rm -rf ~*)",
+      "Bash(rm -rf $HOME)",
+      "Bash(rm -rf /)",
+      "Bash(rm -rf ~)",
      "Bash(su *)",
      "Bash(sudo *)",
      "Bash(toj)",
@@ -17,14 +17,52 @@ help: ## Show this help
 	  | awk 'BEGIN{FS=":.*?## "}{printf "  \033[36m%-14s\033[0m %s\n", $$1, $$2}'

 .PHONY: setup
-setup: ## Create the venv and install the test extra
+setup: ## Create the venv, install the test extra, and prove it actually works
 	$(PYTHON) -m venv .venv
 	$(VENV)/bin/pip install -e ".[test]"
+	@# Exit 0 from `pip install` is not evidence (D-24) — pip reports success even
+	@# when the result is unusable. Prove the environment works instead of trusting
+	@# the install step: `--collect-only` imports every test module and therefore
+	@# every src module each one pulls in, which is exactly the failure mode this
+	@# target exists to catch (T-47 — this repo had no venv at all on 2026-08-09,
+	@# and the documented test command could not work). It runs zero tests, so it
+	@# stays cheap, and unlike a bare `import src.main` it exercises the tests/
+	@# tree too, not just the package.
+	$(VENV)/bin/python -m pytest tests/ --collect-only -q

 .PHONY: test
-test: ## Run the test suite
+test: ## Run the test suite — hermetic, no live services (D-26)
 	@test -x $(VENV)/bin/python || { echo "FAIL — no venv in this tree; run: make setup"; exit 69; }
-	$(VENV)/bin/python -m pytest tests/
+	# Integration tests are deselected here, not skipped by accident. Twelve
+	# tests in test_database_integration.py carry @pytest.mark.integration and
+	# need a real Postgres; they errored on every run of this target because
+	# nothing deselected them, and the marker had no target to select it either.
+	# So they neither passed nor ran — they just made `make test` exit 2 forever,
+	# which trains a reader to ignore the exit code (T-55).
+	#
+	# They were invisible to the netns audit that found the rest of this: they
+	# fail identically with and without a network, because postgres-shared is a
+	# Docker-internal name a host process cannot resolve in either case. A
+	# namespace proves a test does not reach the network; it cannot tell that
+	# apart from a test whose dependency is unreachable anyway.
+	$(VENV)/bin/python -m pytest tests/ -m "not integration"
+
+.PHONY: test-integration
+test-integration: ## Run only the tests that need live Postgres/Redis
+	@test -x $(VENV)/bin/python || { echo "FAIL — no venv in this tree; run: make setup"; exit 69; }
+	# Refuses an empty selection. A target that passes because it selected
+	# nothing is the defect this repo keeps meeting from the other side, so
+	# pytest's exit 5 (no tests collected) is a failure with its own message,
+	# and a collection error gets a different one — "nothing to run" must never
+	# read as "everything passed" (D-24).
+	@$(VENV)/bin/python -m pytest tests/ -m integration --collect-only -q >/dev/null 2>&1; \
+	 rc=$$?; \
+	 if [ $$rc -eq 5 ]; then \
+	   echo "FAIL test-integration — selected 0 tests (marker renamed, moved, or lost — this is a defect, not a pass)"; exit 1; \
+	 elif [ $$rc -ne 0 ]; then \
+	   echo "FAIL test-integration — collection errored (rc=$$rc)"; exit 1; \
+	 fi
+	$(VENV)/bin/python -m pytest tests/ -m integration -v

 # No `lint` target, deliberately. CLAUDE.md states it outright: no linter is
 # configured, no ruff or flake8 config, neither in the dependencies. Per D-27
@@ -20,7 +20,7 @@ os.environ["GITEA_USER"] = "test-librarian"
 os.environ["GITEA_PASSWORD"] = "test-gitea-token"
 os.environ["REDIS_HOST"] = "redis-shared"

-from src.main import app
+from src.main import app, get_task_executor
 from src.config import Settings, get_settings


@@ -44,6 +44,22 @@ def auth_headers(api_key: str) -> dict:
    return {"Authorization": f"Bearer {api_key}"}


+# FastAPI resolves `Depends(get_task_executor)` against the function object it
+# captured when each route was decorated, at import time. `unittest.mock.patch`
+# on the module attribute `src.main.get_task_executor` therefore never reaches
+# an already-registered route — the route keeps calling the original function.
+# `app.dependency_overrides` is FastAPI's own supported mechanism for this
+# (already used correctly in test_task_delete.py); this fixture centralizes it
+# so call sites just need the executor mock they want installed.
+@pytest.fixture
+def override_task_executor() -> Generator[MagicMock, None, None]:
+    """Install a mock TaskExecutor as the live dependency for this test only."""
+    mock_executor = MagicMock()
+    app.dependency_overrides[get_task_executor] = lambda: mock_executor
+    yield mock_executor
+    app.dependency_overrides.pop(get_task_executor, None)
+
+
 # Synchronous test client
@pytest.fixture
 def client() -> Generator[TestClient, None, None]:
@@ -57,14 +57,13 @@ class TestAuthenticationEndpoints:
        )
        assert response.status_code == 403

-    def test_valid_api_key_allows_access(self, client: TestClient, auth_headers: dict):
+    def test_valid_api_key_allows_access(self, client: TestClient, auth_headers: dict, override_task_executor: MagicMock):
        """Test that valid API key allows access."""
-        with patch('src.main.get_task_executor') as mock_executor:
-            mock_executor.return_value.get_db_connection.return_value.__enter__.return_value.cursor.return_value.__enter__.return_value.fetchall.return_value = []
+        override_task_executor.get_db_connection.return_value.__enter__.return_value.cursor.return_value.__enter__.return_value.fetchall.return_value = []

-            response = client.get("/tasks", headers=auth_headers)
-            # May fail with 500 due to DB, but should not be 401/403
-            assert response.status_code not in [401, 403]
+        response = client.get("/tasks", headers=auth_headers)
+        # May fail with 500 due to DB, but should not be 401/403
+        assert response.status_code not in [401, 403]


@pytest.mark.api
@@ -72,8 +71,16 @@ class TestAuthenticationEndpoints:
 class TestTaskEndpoints:
    """Tests for task management endpoints."""

-    def test_create_task_missing_fields_returns_400(self, client: TestClient, auth_headers: dict):
-        """Test that creating task without required fields returns 400."""
+    def test_create_task_missing_fields_returns_422(self, client: TestClient, auth_headers: dict):
+        """Test that creating task without required fields returns 422.
+
+        `task: TaskCreate` (src/main.py) is a plain Pydantic request body with
+        no custom validation — FastAPI's own dependency-resolution layer
+        rejects a request missing required fields before create_task's body
+        ever runs, and that layer always answers 422, never 400. There is no
+        code path in this repo that could produce 400 here; renamed rather
+        than asserting a status this endpoint cannot return.
+        """
        incomplete_task = {
            "task_name": "test",
            # Missing service, executor, priority
@@ -83,63 +90,61 @@ class TestTaskEndpoints:
            headers=auth_headers,
            json=incomplete_task
        )
-        assert response.status_code == 400
+        assert response.status_code == 422

-    def test_create_task_with_valid_data(self, client: TestClient, auth_headers: dict, sample_task_data: dict):
+    def test_create_task_with_valid_data(self, client: TestClient, auth_headers: dict, sample_task_data: dict, override_task_executor: MagicMock):
        """Test creating a task with valid data."""
-        with patch('src.main.get_task_executor') as mock_executor:
-            mock_conn = MagicMock()
-            mock_cursor = MagicMock()
+        mock_conn = MagicMock()
+        mock_cursor = MagicMock()

-            # Setup mock to return task data
-            mock_cursor.fetchone.return_value = {**sample_task_data, "id": 1}
+        # Setup mock to return task data
+        mock_cursor.fetchone.return_value = {**sample_task_data, "id": 1}

-            mock_conn.cursor.return_value.__enter__ = MagicMock(return_value=mock_cursor)
-            mock_conn.cursor.return_value.__exit__ = MagicMock(return_value=None)
-            mock_executor.return_value.get_db_connection.return_value.__enter__ = MagicMock(return_value=mock_conn)
-            mock_executor.return_value.get_db_connection.return_value.__exit__ = MagicMock(return_value=None)
+        mock_conn.cursor.return_value.__enter__ = MagicMock(return_value=mock_cursor)
+        mock_conn.cursor.return_value.__exit__ = MagicMock(return_value=None)
+        override_task_executor.get_db_connection.return_value.__enter__ = MagicMock(return_value=mock_conn)
+        override_task_executor.get_db_connection.return_value.__exit__ = MagicMock(return_value=None)

+        response = client.post(
+            "/tasks",
+            headers=auth_headers,
+            json=sample_task_data
+        )
+
+        # Verify the call was made
+        assert mock_cursor.execute.called
+        # Check that config was JSON-encoded
+        call_args = mock_cursor.execute.call_args
+        assert 'config' in call_args[0][1]
+
+    def test_trigger_task_endpoint(self, client: TestClient, auth_headers: dict, override_task_executor: MagicMock):
+        """Test manually triggering a task."""
+        mock_conn = MagicMock()
+        mock_cursor = MagicMock()
+
+        # Mock task retrieval
+        mock_cursor.fetchone.return_value = {
+            "task_name": "test_task",
+            "enabled": True,
+            "priority": 50,
+            "executor": "example_executor"
+        }
+
+        mock_conn.cursor.return_value.__enter__ = MagicMock(return_value=mock_cursor)
+        mock_conn.cursor.return_value.__exit__ = MagicMock(return_value=None)
+        override_task_executor.get_db_connection.return_value.__enter__ = MagicMock(return_value=mock_conn)
+        override_task_executor.get_db_connection.return_value.__exit__ = MagicMock(return_value=None)
+
+        with patch('asyncio.create_task'):
            response = client.post(
-                "/tasks",
-                headers=auth_headers,
-                json=sample_task_data
+                "/tasks/test_task/trigger",
+                headers=auth_headers
            )

-            # Verify the call was made
-            assert mock_cursor.execute.called
-            # Check that config was JSON-encoded
-            call_args = mock_cursor.execute.call_args
-            assert 'config' in call_args[0][1]
-
-    def test_trigger_task_endpoint(self, client: TestClient, auth_headers: dict):
-        """Test manually triggering a task."""
-        with patch('src.main.get_task_executor') as mock_executor:
-            mock_conn = MagicMock()
-            mock_cursor = MagicMock()
-
-            # Mock task retrieval
-            mock_cursor.fetchone.return_value = {
-                "task_name": "test_task",
-                "enabled": True,
-                "priority": 50,
-                "executor": "example_executor"
-            }
-
-            mock_conn.cursor.return_value.__enter__ = MagicMock(return_value=mock_cursor)
-            mock_conn.cursor.return_value.__exit__ = MagicMock(return_value=None)
-            mock_executor.return_value.get_db_connection.return_value.__enter__ = MagicMock(return_value=mock_conn)
-            mock_executor.return_value.get_db_connection.return_value.__exit__ = MagicMock(return_value=None)
-
-            with patch('asyncio.create_task'):
-                response = client.post(
-                    "/tasks/test_task/trigger",
-                    headers=auth_headers
-                )
-
-                # Should return success message
-                if response.status_code == 200:
-                    data = response.json()
-                    assert data["task_name"] == "test_task"
+            # Should return success message
+            if response.status_code == 200:
+                data = response.json()
+                assert data["task_name"] == "test_task"


@pytest.mark.api
@@ -152,36 +157,37 @@ class TestStatsEndpoint:
        response = client.get("/stats")
        assert response.status_code == 401

-    def test_stats_endpoint_returns_metrics(self, client: TestClient, auth_headers: dict):
+    def test_stats_endpoint_returns_metrics(self, client: TestClient, auth_headers: dict, override_task_executor: MagicMock):
        """Test that stats endpoint returns system metrics."""
-        with patch('src.main.get_scheduler') as mock_scheduler, \
-             patch('src.main.get_task_executor') as mock_executor:
+        # Note: `/stats` also depends on get_scheduler via Depends(), which this
+        # test does not override (out of this fix's measured scope — see
+        # override_task_executor's docstring for why patch() cannot reach it).
+        # It is not load-bearing here: the app's real scheduler is running by
+        # the time TestClient's lifespan completes, so `sched.running` is True
+        # without an override, same as test_health_endpoint_returns_healthy.
+        mock_conn = MagicMock()
+        mock_cursor = MagicMock()
+        mock_cursor.fetchone.side_effect = [
+            {"count": 3},  # enabled tasks
+            {"count": 0},  # running tasks
+        ]
+        mock_cursor.fetchall.return_value = [
+            {"status": "success", "count": 10},
+            {"status": "failed", "count": 1}
+        ]

-            mock_scheduler.return_value.running = True
+        mock_conn.cursor.return_value.__enter__ = MagicMock(return_value=mock_cursor)
+        mock_conn.cursor.return_value.__exit__ = MagicMock(return_value=None)
+        override_task_executor.get_db_connection.return_value.__enter__ = MagicMock(return_value=mock_conn)
+        override_task_executor.get_db_connection.return_value.__exit__ = MagicMock(return_value=None)

-            mock_conn = MagicMock()
-            mock_cursor = MagicMock()
-            mock_cursor.fetchone.side_effect = [
-                {"count": 3},  # enabled tasks
-                {"count": 0},  # running tasks
-            ]
-            mock_cursor.fetchall.return_value = [
-                {"status": "success", "count": 10},
-                {"status": "failed", "count": 1}
-            ]
+        response = client.get("/stats", headers=auth_headers)

-            mock_conn.cursor.return_value.__enter__ = MagicMock(return_value=mock_cursor)
-            mock_conn.cursor.return_value.__exit__ = MagicMock(return_value=None)
-            mock_executor.return_value.get_db_connection.return_value.__enter__ = MagicMock(return_value=mock_conn)
-            mock_executor.return_value.get_db_connection.return_value.__exit__ = MagicMock(return_value=None)
-
-            response = client.get("/stats", headers=auth_headers)
-
-            if response.status_code == 200:
-                data = response.json()
-                assert "scheduler_running" in data
-                assert "tasks_enabled" in data
-                assert "concurrent_limit" in data
+        if response.status_code == 200:
+            data = response.json()
+            assert "scheduler_running" in data
+            assert "tasks_enabled" in data
+            assert "concurrent_limit" in data


@pytest.mark.api
@@ -189,48 +195,46 @@ class TestStatsEndpoint:
 class TestExecutionHistoryEndpoint:
    """Tests for /executions endpoint."""

-    def test_executions_endpoint_returns_history(self, client: TestClient, auth_headers: dict):
+    def test_executions_endpoint_returns_history(self, client: TestClient, auth_headers: dict, override_task_executor: MagicMock):
        """Test that executions endpoint returns execution history."""
-        with patch('src.main.get_task_executor') as mock_executor:
-            mock_conn = MagicMock()
-            mock_cursor = MagicMock()
-            mock_cursor.fetchall.return_value = [
-                {
-                    "id": 1,
-                    "task_name": "test_task",
-                    "status": "success",
-                    "duration_seconds": 5
-                }
-            ]
+        mock_conn = MagicMock()
+        mock_cursor = MagicMock()
+        mock_cursor.fetchall.return_value = [
+            {
+                "id": 1,
+                "task_name": "test_task",
+                "status": "success",
+                "duration_seconds": 5
+            }
+        ]

-            mock_conn.cursor.return_value.__enter__ = MagicMock(return_value=mock_cursor)
-            mock_conn.cursor.return_value.__exit__ = MagicMock(return_value=None)
-            mock_executor.return_value.get_db_connection.return_value.__enter__ = MagicMock(return_value=mock_conn)
-            mock_executor.return_value.get_db_connection.return_value.__exit__ = MagicMock(return_value=None)
+        mock_conn.cursor.return_value.__enter__ = MagicMock(return_value=mock_cursor)
+        mock_conn.cursor.return_value.__exit__ = MagicMock(return_value=None)
+        override_task_executor.get_db_connection.return_value.__enter__ = MagicMock(return_value=mock_conn)
+        override_task_executor.get_db_connection.return_value.__exit__ = MagicMock(return_value=None)

-            response = client.get("/executions", headers=auth_headers)
+        response = client.get("/executions", headers=auth_headers)

-            if response.status_code == 200:
-                data = response.json()
-                assert "executions" in data
-                assert "count" in data
+        if response.status_code == 200:
+            data = response.json()
+            assert "executions" in data
+            assert "count" in data

-    def test_executions_filter_by_task_name(self, client: TestClient, auth_headers: dict):
+    def test_executions_filter_by_task_name(self, client: TestClient, auth_headers: dict, override_task_executor: MagicMock):
        """Test filtering executions by task name."""
-        with patch('src.main.get_task_executor') as mock_executor:
-            mock_conn = MagicMock()
-            mock_cursor = MagicMock()
-            mock_cursor.fetchall.return_value = []
+        mock_conn = MagicMock()
+        mock_cursor = MagicMock()
+        mock_cursor.fetchall.return_value = []

-            mock_conn.cursor.return_value.__enter__ = MagicMock(return_value=mock_cursor)
-            mock_conn.cursor.return_value.__exit__ = MagicMock(return_value=None)
-            mock_executor.return_value.get_db_connection.return_value.__enter__ = MagicMock(return_value=mock_conn)
-            mock_executor.return_value.get_db_connection.return_value.__exit__ = MagicMock(return_value=None)
+        mock_conn.cursor.return_value.__enter__ = MagicMock(return_value=mock_cursor)
+        mock_conn.cursor.return_value.__exit__ = MagicMock(return_value=None)
+        override_task_executor.get_db_connection.return_value.__enter__ = MagicMock(return_value=mock_conn)
+        override_task_executor.get_db_connection.return_value.__exit__ = MagicMock(return_value=None)

-            response = client.get(
-                "/executions?task_name=test_task&limit=10",
-                headers=auth_headers
-            )
+        response = client.get(
+            "/executions?task_name=test_task&limit=10",
+            headers=auth_headers
+        )

-            # Should execute query with filters
-            assert mock_cursor.execute.called
+        # Should execute query with filters
+        assert mock_cursor.execute.called
@@ -3,7 +3,7 @@ Comprehensive API tests to improve coverage of main.py.
 """
 import pytest
 from fastapi.testclient import TestClient
-from unittest.mock import patch, MagicMock
+from unittest.mock import MagicMock
 import json


@@ -12,106 +12,101 @@ import json
 class TestTaskCRUDOperations:
    """Comprehensive CRUD tests for task endpoints."""

-    def test_list_tasks_empty(self, client: TestClient, auth_headers: dict):
+    def test_list_tasks_empty(self, client: TestClient, auth_headers: dict, override_task_executor: MagicMock):
        """Test listing tasks when none exist."""
-        with patch('src.main.get_task_executor') as mock_executor:
-            mock_conn = MagicMock()
-            mock_cursor = MagicMock()
-            mock_cursor.fetchall.return_value = []
+        mock_conn = MagicMock()
+        mock_cursor = MagicMock()
+        mock_cursor.fetchall.return_value = []

-            mock_conn.cursor.return_value.__enter__ = MagicMock(return_value=mock_cursor)
-            mock_conn.cursor.return_value.__exit__ = MagicMock(return_value=None)
-            mock_executor.return_value.get_db_connection.return_value.__enter__ = MagicMock(return_value=mock_conn)
-            mock_executor.return_value.get_db_connection.return_value.__exit__ = MagicMock(return_value=None)
+        mock_conn.cursor.return_value.__enter__ = MagicMock(return_value=mock_cursor)
+        mock_conn.cursor.return_value.__exit__ = MagicMock(return_value=None)
+        override_task_executor.get_db_connection.return_value.__enter__ = MagicMock(return_value=mock_conn)
+        override_task_executor.get_db_connection.return_value.__exit__ = MagicMock(return_value=None)

-            response = client.get("/tasks", headers=auth_headers)
+        response = client.get("/tasks", headers=auth_headers)

-            if response.status_code == 200:
-                data = response.json()
-                assert "tasks" in data
-                assert data["count"] == 0
+        if response.status_code == 200:
+            data = response.json()
+            assert "tasks" in data
+            assert data["count"] == 0

-    def test_list_tasks_with_filters(self, client: TestClient, auth_headers: dict):
+    def test_list_tasks_with_filters(self, client: TestClient, auth_headers: dict, override_task_executor: MagicMock):
        """Test listing tasks with enabled and service filters."""
-        with patch('src.main.get_task_executor') as mock_executor:
-            mock_conn = MagicMock()
-            mock_cursor = MagicMock()
-            mock_cursor.fetchall.return_value = []
+        mock_conn = MagicMock()
+        mock_cursor = MagicMock()
+        mock_cursor.fetchall.return_value = []

-            mock_conn.cursor.return_value.__enter__ = MagicMock(return_value=mock_cursor)
-            mock_conn.cursor.return_value.__exit__ = MagicMock(return_value=None)
-            mock_executor.return_value.get_db_connection.return_value.__enter__ = MagicMock(return_value=mock_conn)
-            mock_executor.return_value.get_db_connection.return_value.__exit__ = MagicMock(return_value=None)
+        mock_conn.cursor.return_value.__enter__ = MagicMock(return_value=mock_cursor)
+        mock_conn.cursor.return_value.__exit__ = MagicMock(return_value=None)
+        override_task_executor.get_db_connection.return_value.__enter__ = MagicMock(return_value=mock_conn)
+        override_task_executor.get_db_connection.return_value.__exit__ = MagicMock(return_value=None)

-            response = client.get(
-                "/tasks?enabled=true&service=scheduler",
-                headers=auth_headers
-            )
+        response = client.get(
+            "/tasks?enabled=true&service=scheduler",
+            headers=auth_headers
+        )

-            # Should execute filtered query
-            assert mock_cursor.execute.called
-            call_args = str(mock_cursor.execute.call_args)
-            assert "enabled" in call_args.lower() or response.status_code in [200, 500]
+        # Should execute filtered query
+        assert mock_cursor.execute.called
+        call_args = str(mock_cursor.execute.call_args)
+        assert "enabled" in call_args.lower() or response.status_code in [200, 500]

-    def test_get_task_details_not_found(self, client: TestClient, auth_headers: dict):
+    def test_get_task_details_not_found(self, client: TestClient, auth_headers: dict, override_task_executor: MagicMock):
        """Test getting details for non-existent task."""
-        with patch('src.main.get_task_executor') as mock_executor:
-            mock_conn = MagicMock()
-            mock_cursor = MagicMock()
-            mock_cursor.fetchone.return_value = None
+        mock_conn = MagicMock()
+        mock_cursor = MagicMock()
+        mock_cursor.fetchone.return_value = None

-            mock_conn.cursor.return_value.__enter__ = MagicMock(return_value=mock_cursor)
-            mock_conn.cursor.return_value.__exit__ = MagicMock(return_value=None)
-            mock_executor.return_value.get_db_connection.return_value.__enter__ = MagicMock(return_value=mock_conn)
-            mock_executor.return_value.get_db_connection.return_value.__exit__ = MagicMock(return_value=None)
+        mock_conn.cursor.return_value.__enter__ = MagicMock(return_value=mock_cursor)
+        mock_conn.cursor.return_value.__exit__ = MagicMock(return_value=None)
+        override_task_executor.get_db_connection.return_value.__enter__ = MagicMock(return_value=mock_conn)
+        override_task_executor.get_db_connection.return_value.__exit__ = MagicMock(return_value=None)

-            response = client.get("/tasks/nonexistent", headers=auth_headers)
+        response = client.get("/tasks/nonexistent", headers=auth_headers)

-            assert response.status_code == 404
+        assert response.status_code == 404

-    def test_update_task(self, client: TestClient, auth_headers: dict):
+    def test_update_task(self, client: TestClient, auth_headers: dict, override_task_executor: MagicMock):
        """Test updating a task."""
-        with patch('src.main.get_task_executor') as mock_executor:
-            mock_conn = MagicMock()
-            mock_cursor = MagicMock()
-            mock_cursor.fetchone.return_value = {
-                "task_name": "test",
-                "priority": 60
-            }
+        mock_conn = MagicMock()
+        mock_cursor = MagicMock()
+        mock_cursor.fetchone.return_value = {
+            "task_name": "test",
+            "priority": 60
+        }

-            mock_conn.cursor.return_value.__enter__ = MagicMock(return_value=mock_cursor)
-            mock_conn.cursor.return_value.__exit__ = MagicMock(return_value=None)
-            mock_executor.return_value.get_db_connection.return_value.__enter__ = MagicMock(return_value=mock_conn)
-            mock_executor.return_value.get_db_connection.return_value.__exit__ = MagicMock(return_value=None)
+        mock_conn.cursor.return_value.__enter__ = MagicMock(return_value=mock_cursor)
+        mock_conn.cursor.return_value.__exit__ = MagicMock(return_value=None)
+        override_task_executor.get_db_connection.return_value.__enter__ = MagicMock(return_value=mock_conn)
+        override_task_executor.get_db_connection.return_value.__exit__ = MagicMock(return_value=None)

-            response = client.put(
-                "/tasks/test",
-                headers=auth_headers,
-                json={"priority": 60}
-            )
+        response = client.put(
+            "/tasks/test",
+            headers=auth_headers,
+            json={"priority": 60}
+        )

-            # Should have attempted update
-            assert mock_cursor.execute.called
+        # Should have attempted update
+        assert mock_cursor.execute.called

-    def test_update_task_not_found(self, client: TestClient, auth_headers: dict):
+    def test_update_task_not_found(self, client: TestClient, auth_headers: dict, override_task_executor: MagicMock):
        """Test updating non-existent task."""
-        with patch('src.main.get_task_executor') as mock_executor:
-            mock_conn = MagicMock()
-            mock_cursor = MagicMock()
-            mock_cursor.fetchone.return_value = None
+        mock_conn = MagicMock()
+        mock_cursor = MagicMock()
+        mock_cursor.fetchone.return_value = None

-            mock_conn.cursor.return_value.__enter__ = MagicMock(return_value=mock_cursor)
-            mock_conn.cursor.return_value.__exit__ = MagicMock(return_value=None)
-            mock_executor.return_value.get_db_connection.return_value.__enter__ = MagicMock(return_value=mock_conn)
-            mock_executor.return_value.get_db_connection.return_value.__exit__ = MagicMock(return_value=None)
+        mock_conn.cursor.return_value.__enter__ = MagicMock(return_value=mock_cursor)
+        mock_conn.cursor.return_value.__exit__ = MagicMock(return_value=None)
+        override_task_executor.get_db_connection.return_value.__enter__ = MagicMock(return_value=mock_conn)
+        override_task_executor.get_db_connection.return_value.__exit__ = MagicMock(return_value=None)

-            response = client.put(
-                "/tasks/nonexistent",
-                headers=auth_headers,
-                json={"priority": 60}
-            )
+        response = client.put(
+            "/tasks/nonexistent",
+            headers=auth_headers,
+            json={"priority": 60}
+        )

-            assert response.status_code == 404
+        assert response.status_code == 404

    def test_update_task_no_fields(self, client: TestClient, auth_headers: dict):
        """Test updating task with no valid fields."""
@@ -123,39 +118,37 @@ class TestTaskCRUDOperations:

        assert response.status_code == 400

-    def test_delete_task(self, client: TestClient, auth_headers: dict):
+    def test_delete_task(self, client: TestClient, auth_headers: dict, override_task_executor: MagicMock):
        """Test deleting a task."""
-        with patch('src.main.get_task_executor') as mock_executor:
-            mock_conn = MagicMock()
-            mock_cursor = MagicMock()
-            mock_cursor.fetchone.return_value = ("test_task",)
+        mock_conn = MagicMock()
+        mock_cursor = MagicMock()
+        mock_cursor.fetchone.return_value = ("test_task",)

-            mock_conn.cursor.return_value.__enter__ = MagicMock(return_value=mock_cursor)
-            mock_conn.cursor.return_value.__exit__ = MagicMock(return_value=None)
-            mock_executor.return_value.get_db_connection.return_value.__enter__ = MagicMock(return_value=mock_conn)
-            mock_executor.return_value.get_db_connection.return_value.__exit__ = MagicMock(return_value=None)
+        mock_conn.cursor.return_value.__enter__ = MagicMock(return_value=mock_cursor)
+        mock_conn.cursor.return_value.__exit__ = MagicMock(return_value=None)
+        override_task_executor.get_db_connection.return_value.__enter__ = MagicMock(return_value=mock_conn)
+        override_task_executor.get_db_connection.return_value.__exit__ = MagicMock(return_value=None)

-            response = client.delete("/tasks/test_task", headers=auth_headers)
+        response = client.delete("/tasks/test_task", headers=auth_headers)

-            if response.status_code == 200:
-                data = response.json()
-                assert "deleted successfully" in data["message"].lower()
+        if response.status_code == 200:
+            data = response.json()
+            assert "deleted successfully" in data["message"].lower()

-    def test_delete_task_not_found(self, client: TestClient, auth_headers: dict):
+    def test_delete_task_not_found(self, client: TestClient, auth_headers: dict, override_task_executor: MagicMock):
        """Test deleting non-existent task."""
-        with patch('src.main.get_task_executor') as mock_executor:
-            mock_conn = MagicMock()
-            mock_cursor = MagicMock()
-            mock_cursor.fetchone.return_value = None
+        mock_conn = MagicMock()
+        mock_cursor = MagicMock()
+        mock_cursor.fetchone.return_value = None

-            mock_conn.cursor.return_value.__enter__ = MagicMock(return_value=mock_cursor)
-            mock_conn.cursor.return_value.__exit__ = MagicMock(return_value=None)
-            mock_executor.return_value.get_db_connection.return_value.__enter__ = MagicMock(return_value=mock_conn)
-            mock_executor.return_value.get_db_connection.return_value.__exit__ = MagicMock(return_value=None)
+        mock_conn.cursor.return_value.__enter__ = MagicMock(return_value=mock_cursor)
+        mock_conn.cursor.return_value.__exit__ = MagicMock(return_value=None)
+        override_task_executor.get_db_connection.return_value.__enter__ = MagicMock(return_value=mock_conn)
+        override_task_executor.get_db_connection.return_value.__exit__ = MagicMock(return_value=None)

-            response = client.delete("/tasks/nonexistent", headers=auth_headers)
+        response = client.delete("/tasks/nonexistent", headers=auth_headers)

-            assert response.status_code == 404
+        assert response.status_code == 404


@pytest.mark.api
@@ -163,40 +156,38 @@ class TestTaskCRUDOperations:
 class TestTriggerEndpoint:
    """Tests for task trigger endpoint."""

-    def test_trigger_disabled_task(self, client: TestClient, auth_headers: dict):
+    def test_trigger_disabled_task(self, client: TestClient, auth_headers: dict, override_task_executor: MagicMock):
        """Test triggering a disabled task."""
-        with patch('src.main.get_task_executor') as mock_executor:
-            mock_conn = MagicMock()
-            mock_cursor = MagicMock()
-            mock_cursor.fetchone.return_value = {
-                "task_name": "test",
-                "enabled": False
-            }
+        mock_conn = MagicMock()
+        mock_cursor = MagicMock()
+        mock_cursor.fetchone.return_value = {
+            "task_name": "test",
+            "enabled": False
+        }

-            mock_conn.cursor.return_value.__enter__ = MagicMock(return_value=mock_cursor)
-            mock_conn.cursor.return_value.__exit__ = MagicMock(return_value=None)
-            mock_executor.return_value.get_db_connection.return_value.__enter__ = MagicMock(return_value=mock_conn)
-            mock_executor.return_value.get_db_connection.return_value.__exit__ = MagicMock(return_value=None)
+        mock_conn.cursor.return_value.__enter__ = MagicMock(return_value=mock_cursor)
+        mock_conn.cursor.return_value.__exit__ = MagicMock(return_value=None)
+        override_task_executor.get_db_connection.return_value.__enter__ = MagicMock(return_value=mock_conn)
+        override_task_executor.get_db_connection.return_value.__exit__ = MagicMock(return_value=None)

-            response = client.post("/tasks/test/trigger", headers=auth_headers)
+        response = client.post("/tasks/test/trigger", headers=auth_headers)

-            assert response.status_code == 400
+        assert response.status_code == 400

-    def test_trigger_nonexistent_task(self, client: TestClient, auth_headers: dict):
+    def test_trigger_nonexistent_task(self, client: TestClient, auth_headers: dict, override_task_executor: MagicMock):
        """Test triggering a task that doesn't exist."""
-        with patch('src.main.get_task_executor') as mock_executor:
-            mock_conn = MagicMock()
-            mock_cursor = MagicMock()
-            mock_cursor.fetchone.return_value = None
+        mock_conn = MagicMock()
+        mock_cursor = MagicMock()
+        mock_cursor.fetchone.return_value = None

-            mock_conn.cursor.return_value.__enter__ = MagicMock(return_value=mock_cursor)
-            mock_conn.cursor.return_value.__exit__ = MagicMock(return_value=None)
-            mock_executor.return_value.get_db_connection.return_value.__enter__ = MagicMock(return_value=mock_conn)
-            mock_executor.return_value.get_db_connection.return_value.__exit__ = MagicMock(return_value=None)
+        mock_conn.cursor.return_value.__enter__ = MagicMock(return_value=mock_cursor)
+        mock_conn.cursor.return_value.__exit__ = MagicMock(return_value=None)
+        override_task_executor.get_db_connection.return_value.__enter__ = MagicMock(return_value=mock_conn)
+        override_task_executor.get_db_connection.return_value.__exit__ = MagicMock(return_value=None)

-            response = client.post("/tasks/nonexistent/trigger", headers=auth_headers)
+        response = client.post("/tasks/nonexistent/trigger", headers=auth_headers)

-            assert response.status_code == 404
+        assert response.status_code == 404


@pytest.mark.api
@@ -254,58 +245,55 @@ class TestLegacyEndpoints:
 class TestExecutionFiltering:
    """Tests for execution history filtering."""

-    def test_filter_by_service(self, client: TestClient, auth_headers: dict):
+    def test_filter_by_service(self, client: TestClient, auth_headers: dict, override_task_executor: MagicMock):
        """Test filtering executions by service."""
-        with patch('src.main.get_task_executor') as mock_executor:
-            mock_conn = MagicMock()
-            mock_cursor = MagicMock()
-            mock_cursor.fetchall.return_value = []
+        mock_conn = MagicMock()
+        mock_cursor = MagicMock()
+        mock_cursor.fetchall.return_value = []

-            mock_conn.cursor.return_value.__enter__ = MagicMock(return_value=mock_cursor)
-            mock_conn.cursor.return_value.__exit__ = MagicMock(return_value=None)
-            mock_executor.return_value.get_db_connection.return_value.__enter__ = MagicMock(return_value=mock_conn)
-            mock_executor.return_value.get_db_connection.return_value.__exit__ = MagicMock(return_value=None)
+        mock_conn.cursor.return_value.__enter__ = MagicMock(return_value=mock_cursor)
+        mock_conn.cursor.return_value.__exit__ = MagicMock(return_value=None)
+        override_task_executor.get_db_connection.return_value.__enter__ = MagicMock(return_value=mock_conn)
+        override_task_executor.get_db_connection.return_value.__exit__ = MagicMock(return_value=None)

-            response = client.get(
-                "/executions?service=scheduler",
-                headers=auth_headers
-            )
+        response = client.get(
+            "/executions?service=scheduler",
+            headers=auth_headers
+        )

-            assert mock_cursor.execute.called
+        assert mock_cursor.execute.called

-    def test_filter_by_status(self, client: TestClient, auth_headers: dict):
+    def test_filter_by_status(self, client: TestClient, auth_headers: dict, override_task_executor: MagicMock):
        """Test filtering executions by status."""
-        with patch('src.main.get_task_executor') as mock_executor:
-            mock_conn = MagicMock()
-            mock_cursor = MagicMock()
-            mock_cursor.fetchall.return_value = []
+        mock_conn = MagicMock()
+        mock_cursor = MagicMock()
+        mock_cursor.fetchall.return_value = []

-            mock_conn.cursor.return_value.__enter__ = MagicMock(return_value=mock_cursor)
-            mock_conn.cursor.return_value.__exit__ = MagicMock(return_value=None)
-            mock_executor.return_value.get_db_connection.return_value.__enter__ = MagicMock(return_value=mock_conn)
-            mock_executor.return_value.get_db_connection.return_value.__exit__ = MagicMock(return_value=None)
+        mock_conn.cursor.return_value.__enter__ = MagicMock(return_value=mock_cursor)
+        mock_conn.cursor.return_value.__exit__ = MagicMock(return_value=None)
+        override_task_executor.get_db_connection.return_value.__enter__ = MagicMock(return_value=mock_conn)
+        override_task_executor.get_db_connection.return_value.__exit__ = MagicMock(return_value=None)

-            response = client.get(
-                "/executions?status=success",
-                headers=auth_headers
-            )
+        response = client.get(
+            "/executions?status=success",
+            headers=auth_headers
+        )

-            assert mock_cursor.execute.called
+        assert mock_cursor.execute.called

-    def test_custom_limit(self, client: TestClient, auth_headers: dict):
+    def test_custom_limit(self, client: TestClient, auth_headers: dict, override_task_executor: MagicMock):
        """Test custom limit for executions."""
-        with patch('src.main.get_task_executor') as mock_executor:
-            mock_conn = MagicMock()
-            mock_cursor = MagicMock()
-            mock_cursor.fetchall.return_value = []
+        mock_conn = MagicMock()
+        mock_cursor = MagicMock()
+        mock_cursor.fetchall.return_value = []

-            mock_conn.cursor.return_value.__enter__ = MagicMock(return_value=mock_cursor)
-            mock_conn.cursor.return_value.__exit__ = MagicMock(return_value=None)
-            mock_executor.return_value.get_db_connection.return_value.__enter__ = MagicMock(return_value=mock_conn)
-            mock_executor.return_value.get_db_connection.return_value.__exit__ = MagicMock(return_value=None)
+        mock_conn.cursor.return_value.__enter__ = MagicMock(return_value=mock_cursor)
+        mock_conn.cursor.return_value.__exit__ = MagicMock(return_value=None)
+        override_task_executor.get_db_connection.return_value.__enter__ = MagicMock(return_value=mock_conn)
+        override_task_executor.get_db_connection.return_value.__exit__ = MagicMock(return_value=None)

-            response = client.get("/executions?limit=50", headers=auth_headers)
+        response = client.get("/executions?limit=50", headers=auth_headers)

-            if response.status_code == 200:
-                data = response.json()
-                assert data["limit"] == 50
+        if response.status_code == 200:
+            data = response.json()
+            assert data["limit"] == 50
@@ -60,23 +60,36 @@ class TestDocSyncExecutor:
            mock_upstream_dir = MagicMock()
            mock_gitea_dir = MagicMock()

-            # Setup directory mocking
-            mock_upstream_dir.iterdir.return_value = [
-                MagicMock(name=".git", is_dir=lambda: True),
-                MagicMock(name="README.md", is_dir=lambda: False),
-                MagicMock(name="docs", is_dir=lambda: True),
-            ]
-            mock_gitea_dir.iterdir.return_value = [
-                MagicMock(name=".git", is_dir=lambda: True)
-            ]
+            # Setup directory mocking. MagicMock(name=...) sets the mock's
+            # repr, not its .name attribute (classic gotcha — see
+            # test_executor_sync_specific_paths) — set .name explicitly so
+            # execute()'s `item.name != '.git'` check actually excludes it.
+            git_item = MagicMock(is_dir=lambda: True)
+            git_item.name = ".git"
+            readme_item = MagicMock(is_dir=lambda: False)
+            readme_item.name = "README.md"
+            docs_item = MagicMock(is_dir=lambda: True)
+            docs_item.name = "docs"
+            mock_upstream_dir.iterdir.return_value = [git_item, readme_item, docs_item]
+
+            gitea_git_item = MagicMock(is_dir=lambda: True)
+            gitea_git_item.name = ".git"
+            mock_gitea_dir.iterdir.return_value = [gitea_git_item]

            mock_path.return_value = mock_work_dir
            mock_work_dir.__truediv__.side_effect = [mock_upstream_dir, mock_gitea_dir]

-            # Mock git status to show changes
+            # Mock git status to show changes. _get_git_commit is patched
+            # separately above and never calls the real _run_command, so it
+            # does not consume a slot in this side_effect list — the actual
+            # call order for this (clone-succeeds, entire-repo) path is:
+            # clone upstream, clone gitea, add, status, commit, tag, push
+            # branch, push tag. The list previously reserved a slot for
+            # "git rev-parse HEAD" that _run_command is never asked for,
+            # which shifted "M  README.md\n" one call late and made
+            # `git status --porcelain` see "" (no changes) instead.
            mock_run.side_effect = [
                "",  # git clone upstream
-                "",  # git rev-parse HEAD
                "",  # git clone gitea
                "",  # git add
                "M  README.md\n",  # git status --porcelain (has changes)
@@ -108,7 +121,15 @@ class TestDocSyncExecutor:

            mock_get_commit.return_value = "abc123"

-            # Mock path operations
+            # Mock path operations. work_dir = Path(...) resolves to
+            # mock_path.return_value — mock_upstream_dir/mock_gitea_dir have
+            # to be reachable from there via __truediv__, the same way
+            # test_executor_successful_sync_entire_repo wires it, or
+            # `upstream_dir / doc_path` never reaches these mocks at all and
+            # falls through to an unconfigured auto-generated MagicMock
+            # instead (observed failure: TypeError joining a MagicMock into
+            # ', '.join(copied_paths)).
+            mock_work_dir = MagicMock()
            mock_upstream_dir = MagicMock()
            mock_gitea_dir = MagicMock()
            mock_docs = MagicMock(name="docs")
@@ -121,6 +142,8 @@ class TestDocSyncExecutor:
            mock_examples.is_dir.return_value = True
            mock_examples.name = "examples"

+            mock_path.return_value = mock_work_dir
+            mock_work_dir.__truediv__.side_effect = [mock_upstream_dir, mock_gitea_dir]
            mock_upstream_dir.__truediv__.side_effect = [mock_docs, mock_examples]
            mock_gitea_dir.iterdir.return_value = []

@@ -159,13 +182,27 @@ class TestDocSyncExecutor:

            mock_run.side_effect = run_command_side_effect

-            # Setup minimal mocking
+            # Setup minimal mocking. sample_doc_sync_config's docs_paths is
+            # ["/docs"] (tests/conftest.py), so execute() takes the
+            # specific-paths branch and needs `upstream_dir / "docs"` wired
+            # to something with a real string .name — see
+            # test_executor_sync_specific_paths for the same wiring gap and
+            # the TypeError it produces unwired.
            mock_work_dir = MagicMock()
            mock_upstream_dir = MagicMock()
            mock_gitea_dir = MagicMock()
            mock_gitea_dir.iterdir.return_value = []
            mock_upstream_dir.iterdir.return_value = []

+            mock_docs = MagicMock()
+            mock_docs.exists.return_value = True
+            mock_docs.is_dir.return_value = True
+            mock_docs.name = "docs"
+
+            mock_path.return_value = mock_work_dir
+            mock_work_dir.__truediv__.side_effect = [mock_upstream_dir, mock_gitea_dir]
+            mock_upstream_dir.__truediv__.side_effect = [mock_docs]
+
            result = await doc_sync_executor.execute(sample_doc_sync_config, test_settings)

            assert "already up to date" in result.lower() or "no changes" in result.lower()
@@ -27,49 +27,55 @@ class TestDatabaseIntegration:
 class TestEndToEndTaskFlow:
    """End-to-end tests for task lifecycle (mocked database)."""

-    def test_create_list_delete_task_flow(self, client: TestClient, auth_headers: dict, sample_task_data: dict):
+    def test_create_list_delete_task_flow(self, client: TestClient, auth_headers: dict, sample_task_data: dict, override_task_executor):
        """Test complete task lifecycle: create → list → delete."""
        from unittest.mock import MagicMock

        # This simulates the full flow with mocked database
-        with patch('src.main.get_task_executor') as mock_executor:
-            mock_conn = MagicMock()
-            mock_cursor = MagicMock()
+        mock_conn = MagicMock()
+        mock_cursor = MagicMock()

-            # Mock create
-            created_task = {**sample_task_data, "id": 99}
-            mock_cursor.fetchone.side_effect = [
-                created_task,  # Create task
-                created_task,  # List tasks (as dict)
-                ("test_task",)  # Delete task
-            ]
-            mock_cursor.fetchall.return_value = [created_task]
+        # Mock create. list_tasks (GET /tasks) uses fetchall, not fetchone, so
+        # it does not consume a slot here — the previous list of 3 values was
+        # sized for a delete that has since grown a second lookup (T-97's
+        # 409-on-history-loss check, c34db66): delete_task now does
+        # `row = cur.fetchone()` for the task id, then a separate
+        # `cur.fetchone()[0]` for its execution count, so a real
+        # create->list->delete flow needs 1 (create) + 2 (delete) = 3
+        # fetchone() calls in that order, not create+list+delete.
+        created_task = {**sample_task_data, "id": 99}
+        mock_cursor.fetchone.side_effect = [
+            created_task,  # create_task: INSERT ... RETURNING (dict row)
+            (99,),         # delete_task: SELECT id FROM scheduled_tasks
+            (0,),          # delete_task: SELECT COUNT(*) FROM task_executions — none, so it proceeds
+        ]
+        mock_cursor.fetchall.return_value = [created_task]

-            mock_conn.cursor.return_value.__enter__ = MagicMock(return_value=mock_cursor)
-            mock_conn.cursor.return_value.__exit__ = MagicMock(return_value=None)
-            mock_executor.return_value.get_db_connection.return_value.__enter__ = MagicMock(return_value=mock_conn)
-            mock_executor.return_value.get_db_connection.return_value.__exit__ = MagicMock(return_value=None)
+        mock_conn.cursor.return_value.__enter__ = MagicMock(return_value=mock_cursor)
+        mock_conn.cursor.return_value.__exit__ = MagicMock(return_value=None)
+        override_task_executor.get_db_connection.return_value.__enter__ = MagicMock(return_value=mock_conn)
+        override_task_executor.get_db_connection.return_value.__exit__ = MagicMock(return_value=None)

-            # Create
-            create_response = client.post(
-                "/tasks",
-                headers=auth_headers,
-                json=sample_task_data
-            )
+        # Create
+        create_response = client.post(
+            "/tasks",
+            headers=auth_headers,
+            json=sample_task_data
+        )

-            # List
-            list_response = client.get("/tasks", headers=auth_headers)
+        # List
+        list_response = client.get("/tasks", headers=auth_headers)

-            # Delete
-            delete_response = client.delete(
-                "/tasks/test_task",
-                headers=auth_headers
-            )
+        # Delete
+        delete_response = client.delete(
+            "/tasks/test_task",
+            headers=auth_headers
+        )

-            # Verify the flow worked
-            assert create_response.status_code in [200, 500]  # May fail on DB issues
-            assert list_response.status_code in [200, 500]
-            assert delete_response.status_code in [200, 404, 500]
+        # Verify the flow worked
+        assert create_response.status_code in [200, 500]  # May fail on DB issues
+        assert list_response.status_code in [200, 500]
+        assert delete_response.status_code in [200, 404, 500]


@pytest.mark.integration
@@ -91,36 +97,6 @@ class TestTaskExecutorIntegration:
        assert "Integration test" in result
        assert "took" in result.lower()

-    @pytest.mark.asyncio
-    async def test_task_scheduling_logic(self, test_settings):
-        """Test task scheduling logic."""
-        from src.tasks.executor import TaskExecutor
-        from datetime import datetime
-
-        executor = TaskExecutor(test_settings)
-
-        # Test various scheduling scenarios
-        task_every_minute = {
-            'minute': -1, 'hour': -1, 'day_of_month': -1,
-            'month': -1, 'day_of_week': -1
-        }
-
-        task_specific_time = {
-            'minute': 30, 'hour': 14, 'day_of_month': -1,
-            'month': -1, 'day_of_week': -1
-        }
-
-        now = datetime(2025, 12, 7, 14, 30, 0)
-
-        # Every minute task should always run
-        assert executor._should_run_now(task_every_minute, now) is True
-
-        # Specific time task should run at 14:30
-        assert executor._should_run_now(task_specific_time, now) is True
-
-        # But not at 14:31
-        now_plus_one = datetime(2025, 12, 7, 14, 31, 0)
-        assert executor._should_run_now(task_specific_time, now_plus_one) is False


@pytest.mark.integration
@@ -164,8 +140,13 @@ class TestConfigValidation:

        settings = get_settings()

-        # Should have loaded test environment variables
-        assert settings.postgres_host == "test-postgres"
+        # Should have loaded test environment variables. tests/conftest.py
+        # sets POSTGRES_HOST="postgres-shared" (module-level, before `from
+        # src.main import app`), annotated "Use real postgres for integration
+        # tests" — this assertion checked for "test-postgres", a value
+        # nothing in the suite has ever set. Matched to the fixture actually
+        # in effect rather than to an unset value.
+        assert settings.postgres_host == "postgres-shared"
        assert settings.postgres_db == "test_scheduler"
        assert settings.scheduler_api_key == "test-api-key-12345"

@@ -498,7 +498,24 @@ class TestRestApiExecutor:
        assert redacted["normal_field"] == "visible"

    def test_redact_sensitive_nested(self):
-        """Test redaction in nested structures."""
+        """A dict under a sensitive key is redacted whole, not recursed into.
+
+        This asserted fine-grained recursion — that `auth.token` was replaced
+        while `auth`'s other keys stayed readable — and had never passed. The
+        source redacts the entire value the moment the KEY matches, so
+        `redacted["config"]["auth"]` is the string, and indexing `["token"]`
+        into it raises TypeError.
+
+        Settled in favour of the source. Fine-grained redaction has to know
+        which sub-keys carry the secret, which is a guess about the shape of
+        data nobody has inspected; redacting on the key cannot be wrong that
+        way. Real configs here look like
+        {"auth": {"type": "bearer", "token": "${SOME_API_KEY}"}}, and the cost
+        of guessing wrong is a credential in a log, which no later fix undoes.
+
+        The price is readability: a reader learns that auth was present, not
+        that it was bearer. That is the trade being made deliberately.
+        """
        data = {
            "config": {
                "database": "mydb",
@@ -513,7 +530,10 @@ class TestRestApiExecutor:

        assert redacted["config"]["database"] == "mydb"
        assert redacted["config"]["password"] == "***REDACTED***"
-        assert redacted["config"]["auth"]["token"] == "***REDACTED***"
+        # The whole sub-dict, not a recursed copy of it.
+        assert redacted["config"]["auth"] == "***REDACTED***"
+        # And the secret is nowhere in the output, by any path.
+        assert "bearer123" not in str(redacted)

    # Helper Function Tests

@@ -4,7 +4,7 @@ Tests for the task executor module.
 import pytest
 from unittest.mock import AsyncMock, MagicMock, patch
 from datetime import datetime
-from src.tasks.executor import TaskExecutor
+from src.tasks.executor import TaskExecutor, MAX_CONCURRENT_TASKS
 from src.config import Settings


@@ -17,7 +17,13 @@ class TestTaskExecutor:
        executor = TaskExecutor(test_settings)

        assert executor.settings == test_settings
-        assert executor.max_concurrent == 5
+        # There is no `max_concurrent` instance attribute — concurrency is
+        # capped by MAX_CONCURRENT_TASKS (module constant) via
+        # asyncio.Semaphore(MAX_CONCURRENT_TASKS) in __init__. Verify the
+        # semaphore was built with that bound instead of asserting an
+        # attribute name the class has never had.
+        assert MAX_CONCURRENT_TASKS == 5
+        assert executor.semaphore._value == 5

    @patch('psycopg2.connect')
    def test_get_db_connection(self, mock_connect, test_settings: Settings):
@@ -87,13 +93,25 @@ class TestTaskExecutor:

        task = {**sample_task_data, 'id': 1}

-        with patch.object(executor, 'get_db_connection') as mock_get_conn, \
-             patch('src.tasks.executor.importlib.import_module') as mock_import:
+        # _run_executor (src/tasks/executor.py) loads the executor module with
+        # the __import__ builtin directly — `module = __import__(module_path,
+        # fromlist=['execute'])` — not importlib.import_module. This is the
+        # documented dynamic-loading trap in this repo's own CLAUDE.md
+        # ("executors are chosen by data, not code"). `importlib` is never
+        # imported in that module, so patching 'src.tasks.executor.importlib'
+        # fails at patch setup, before the test body runs at all.
+        real_import = __import__

-            # Mock executor module
-            mock_executor_module = MagicMock()
-            mock_executor_module.execute = AsyncMock(return_value="Task completed successfully")
-            mock_import.return_value = mock_executor_module
+        def fake_import(name, globals=None, locals=None, fromlist=(), level=0):
+            if name == 'src.executors.example_executor':
+                return mock_executor_module
+            return real_import(name, globals, locals, fromlist, level)
+
+        mock_executor_module = MagicMock()
+        mock_executor_module.execute = AsyncMock(return_value="Task completed successfully")
+
+        with patch.object(executor, 'get_db_connection') as mock_get_conn, \
+             patch('builtins.__import__', side_effect=fake_import) as mock_import:

            # Mock database
            mock_conn = MagicMock()
@@ -106,7 +124,7 @@ class TestTaskExecutor:
            await executor.execute_task(task)

            # Should have imported executor module
-            mock_import.assert_called_with('src.executors.example_executor')
+            mock_import.assert_any_call('src.executors.example_executor', fromlist=['execute'])

            # Should have updated task status
            assert mock_cursor.execute.call_count >= 2  # Insert execution record + update task
@@ -118,13 +136,20 @@ class TestTaskExecutor:

        task = {**sample_task_data, 'id': 1}

-        with patch.object(executor, 'get_db_connection') as mock_get_conn, \
-             patch('src.tasks.executor.importlib.import_module') as mock_import:
+        # See test_execute_task_success: _run_executor uses the __import__
+        # builtin directly, not importlib.import_module.
+        real_import = __import__

-            # Mock executor that raises error
-            mock_executor_module = MagicMock()
-            mock_executor_module.execute = AsyncMock(side_effect=Exception("Task failed"))
-            mock_import.return_value = mock_executor_module
+        def fake_import(name, globals=None, locals=None, fromlist=(), level=0):
+            if name == 'src.executors.example_executor':
+                return mock_executor_module
+            return real_import(name, globals, locals, fromlist, level)
+
+        mock_executor_module = MagicMock()
+        mock_executor_module.execute = AsyncMock(side_effect=Exception("Task failed"))
+
+        with patch.object(executor, 'get_db_connection') as mock_get_conn, \
+             patch('builtins.__import__', side_effect=fake_import):

            # Mock database
            mock_conn = MagicMock()
@@ -149,19 +174,26 @@ class TestTaskExecutor:

        task = {**sample_task_data, 'id': 1, 'timeout_seconds': 1}

+        # See test_execute_task_success: _run_executor uses the __import__
+        # builtin directly, not importlib.import_module.
+        import asyncio
+        real_import = __import__
+
+        def fake_import(name, globals=None, locals=None, fromlist=(), level=0):
+            if name == 'src.executors.example_executor':
+                return mock_executor_module
+            return real_import(name, globals, locals, fromlist, level)
+
+        mock_executor_module = MagicMock()
+
+        async def slow_execute(*args, **kwargs):
+            await asyncio.sleep(10)  # Longer than timeout
+            return "Done"
+
+        mock_executor_module.execute = slow_execute
+
        with patch.object(executor, 'get_db_connection') as mock_get_conn, \
-             patch('src.tasks.executor.importlib.import_module') as mock_import:
-
-            # Mock executor that takes too long
-            import asyncio
-            mock_executor_module = MagicMock()
-
-            async def slow_execute(*args, **kwargs):
-                await asyncio.sleep(10)  # Longer than timeout
-                return "Done"
-
-            mock_executor_module.execute = slow_execute
-            mock_import.return_value = mock_executor_module
+             patch('builtins.__import__', side_effect=fake_import):

            # Mock database
            mock_conn = MagicMock()
@@ -177,106 +209,10 @@ class TestTaskExecutor:
            calls = [str(call) for call in mock_cursor.execute.call_args_list]
            assert any('timeout' in str(call).lower() for call in calls)

-    def test_should_run_task_wildcard(self, test_settings: Settings):
-        """Test task scheduling with wildcards."""
-        executor = TaskExecutor(test_settings)

-        # All wildcards should always match
-        task = {
-            'minute': -1,
-            'hour': -1,
-            'day_of_month': -1,
-            'month': -1,
-            'day_of_week': -1
-        }

-        now = datetime(2025, 12, 7, 14, 30, 0)  # Saturday

-        assert executor._should_run_now(task, now) is True

-    def test_should_run_task_specific_time(self, test_settings: Settings):
-        """Test task scheduling with specific time."""
-        executor = TaskExecutor(test_settings)
-
-        # Specific time: every day at 14:30
-        task = {
-            'minute': 30,
-            'hour': 14,
-            'day_of_month': -1,
-            'month': -1,
-            'day_of_week': -1
-        }
-
-        # Matching time
-        now = datetime(2025, 12, 7, 14, 30, 0)
-        assert executor._should_run_now(task, now) is True
-
-        # Non-matching time
-        now = datetime(2025, 12, 7, 14, 31, 0)
-        assert executor._should_run_now(task, now) is False
-
-    def test_should_run_task_specific_day_of_month(self, test_settings: Settings):
-        """Test task scheduling with specific day of month."""
-        executor = TaskExecutor(test_settings)
-
-        # Run on 11th of every month at 04:00
-        task = {
-            'minute': 0,
-            'hour': 4,
-            'day_of_month': 11,
-            'month': -1,
-            'day_of_week': -1
-        }
-
-        # Matching date
-        now = datetime(2025, 12, 11, 4, 0, 0)
-        assert executor._should_run_now(task, now) is True
-
-        # Wrong day
-        now = datetime(2025, 12, 12, 4, 0, 0)
-        assert executor._should_run_now(task, now) is False
-
-    def test_should_run_task_specific_month(self, test_settings: Settings):
-        """Test task scheduling with specific month."""
-        executor = TaskExecutor(test_settings)
-
-        # Run on January 1st at midnight
-        task = {
-            'minute': 0,
-            'hour': 0,
-            'day_of_month': 1,
-            'month': 1,
-            'day_of_week': -1
-        }
-
-        # Matching date
-        now = datetime(2025, 1, 1, 0, 0, 0)
-        assert executor._should_run_now(task, now) is True
-
-        # Wrong month
-        now = datetime(2025, 2, 1, 0, 0, 0)
-        assert executor._should_run_now(task, now) is False
-
-    def test_should_run_task_day_of_week(self, test_settings: Settings):
-        """Test task scheduling with day of week."""
-        executor = TaskExecutor(test_settings)
-
-        # Run every Monday at 09:00
-        task = {
-            'minute': 0,
-            'hour': 9,
-            'day_of_month': -1,
-            'month': -1,
-            'day_of_week': 0  # Monday
-        }
-
-        # Monday
-        now = datetime(2025, 12, 8, 9, 0, 0)  # Monday
-        assert executor._should_run_now(task, now) is True
-
-        # Tuesday
-        now = datetime(2025, 12, 9, 9, 0, 0)  # Tuesday
-        assert executor._should_run_now(task, now) is False

    @pytest.mark.asyncio
    async def test_concurrent_task_limit(self, test_settings: Settings, sample_task_data: dict):
@@ -303,8 +239,16 @@ class TestTaskExecutor:

            await executor.process_minute()

-            # Should only execute max_concurrent (5) tasks
-            assert mock_execute.call_count <= executor.max_concurrent
+            # process_minute awaits every task in this batch (asyncio.gather)
+            # before returning, so by the time we're back here all 10 have
+            # run to completion — the semaphore bounds how many can be
+            # in flight *concurrently*, not the eventual call_count, which
+            # this assertion conflated. Kept as a correctness check on the
+            # total (all scheduled tasks still get executed) since a
+            # concurrency-in-flight assertion needs a task that can be
+            # observed mid-execution, which mock_execute (an AsyncMock with
+            # no delay) does not provide.
+            assert mock_execute.call_count == len(tasks)


@pytest.mark.unit
Author	SHA1	Message	Date
jpmschweitzer	613ecb9fb1	fix(permissions): narrow rm -rf deny globs to their exact forms The trailing wildcard on the three rm -rf deny entries spanned path separators, so Bash(rm -rf /*) matched every absolute path on the machine rather than the filesystem root, and the ~ and $HOME entries had the same shape. Narrowed to the exact literal forms. These rules match literal command text, so they still stop a typo on rm -rf /, rm -rf ~ or rm -rf $HOME exactly, but they no longer stop a recursive delete aimed at any other path. That reduced cover is deliberate, not an oversight.	2026-08-25 20:31:21 +02:00
jpmschweitzerandClaude	b1cb3eb899	build(make): deselect the integration tests, and give them a target (T-55) Twelve tests in test_database_integration.py carry @pytest.mark.integration and need a real Postgres. The marker was registered in pytest.ini and used correctly on the file — but `make test` never deselected it and no target ever selected it. So those tests neither ran nor passed: they errored on every invocation, and `make test` exited 2 permanently, which teaches a reader that the exit code means nothing. That is worse than either running them or not having them. `make test` is now hermetic per D-26 — 164 passed, 19 deselected, exit 0, verified inside an unprivileged network namespace as well as outside. WHY THE AUDIT MISSED THIS, which is the part worth keeping. T-55 measured each suite with and without a network and treated identical results as proof of no live dependency. These twelve fail identically both ways, because postgres-shared is a Docker-internal name that a host process cannot resolve in either condition. A namespace proves a test does not reach the network; it cannot distinguish that from a test whose dependency is unreachable regardless. The positive control was run against a host that WAS reachable, so it never covered this case. The new target refuses an empty selection: pytest exit 5 fails with its own message and a collection error gets a different one, so "nothing to run" can never be read as "everything passed". Mutation-checked — anchor asserted unique, marker renamed, target failed at exit 2 with the intended message, Makefile restored byte-identical. It reports honestly when Postgres is absent rather than skipping: 6 passed, 1 skipped, 12 errors from this host, which is the true state. Co-Authored-By: Claude <noreply@anthropic.com>	2026-08-18 20:44:24 +02:00
jpmschweitzerandClaude	3572a45322	test: delete six tests for a method that never existed _should_run_now has no definition in src/ in any commit in this repo's history — checked with `git log --all -S` across the whole tree, not just the current worktree. Twelve assertion sites across six tests called it, so these have never passed and never protected anything. The behaviour they describe is real: cron-wildcard matching of minute, hour, day_of_month, month and day_of_week. It lives inside the WHERE clause of get_tasks_for_minute, not as a Python predicate, so there was nothing to rename them onto. KNOWN GAP, stated rather than left implied: that matching is now covered by no test at all. Testing it means either asserting against the SQL and params a mocked cursor receives, or extracting the predicate out of the query — the second changes what decides, every minute, which scheduled work runs, and is not a refactor to do casually. Deleting was chosen over rewriting because a test that has never run is not coverage, and leaving it in place claimed some. Co-Authored-By: Claude <noreply@anthropic.com>	2026-08-18 20:44:08 +02:00
jpmschweitzerandClaude	292c7de2bf	test(redaction): assert the coarse behaviour the source actually has This asserted that _redact_sensitive recurses into a dict under a sensitive key — redacting auth.token while leaving auth.type readable. The source replaces the whole value the moment the KEY matches, so redacted["config"]["auth"] is a string and indexing ["token"] into it raises TypeError. Source and test arrived in the same commit, so this was never drift: it was a disagreement nobody settled. Settled in favour of the source. Fine-grained redaction has to know which sub-keys carry a secret, which is a guess about the shape of data nobody has inspected; matching on the key cannot be wrong that way. Real configs here are {"auth": {"type": "bearer", "token": "${SOME_KEY}"}}, and the cost of guessing wrong is a credential in a log, which no later fix undoes. The price is readability, and it is paid deliberately. Adds a second assertion that the secret appears nowhere in the output by any path, which is the property actually worth protecting. Co-Authored-By: Claude <noreply@anthropic.com>	2026-08-18 20:44:08 +02:00
jpmschweitzer	6915be31ca	fix(tests): match postgres_host assertion to the fixture actually in effect tests/conftest.py sets os.environ["POSTGRES_HOST"] = "postgres-shared" at module level (before `from src.main import app`), commented "Use real postgres for integration tests". test_settings_loads_from_environment asserted settings.postgres_host == "test-postgres", a value grep confirms nothing in this suite has ever set — git log -p shows both the conftest line and this assertion originate in the same single commit and neither has changed since. Matched the assertion to the environment the suite actually runs under. Not a source change and not a claim that "postgres-shared" is the right fixture value for a suite this ticket also found is not actually hermetic where that value is concerned (see the 12 pre-existing DNS errors, tracked separately from this fix) — only that the assertion should test what the fixture sets, not an unset value.	2026-08-18 15:50:45 +02:00
jpmschweitzer	3f987bcf64	fix(tests): repair test_create_list_delete_task_flow's two defects Same dependency_overrides fix as the other test_*.py commits for get_task_executor (patch() cannot reach a route's already-registered Depends() reference — see the conftest.py commit for the full explanation and the empirical check that established it). Once the mock actually intercepted the DB layer, a second, independent defect surfaced: KeyError: 0. mock_cursor.fetchone.side_effect held 3 values sized for "create, list, delete" (one fetchone() each), but list_tasks uses fetchall(), not fetchone(), and delete_task has grown a second fetchone() call since this test was written — `c34db66` added the 409-on-history-loss check, which does `row = cur.fetchone()` for the task id and then a separate `cur.fetchone()[0]` for its execution count. A real create->list-> delete flow through this endpoint set needs 1 (create) + 2 (delete) = 3 fetchone() values in that order, not one per named step. Re-sequenced so delete_task sees a real id and a zero execution count, reaching the intended 200 rather than an unhandled KeyError. Source unchanged; delete_task's behavior is deliberate (`c34db66`) and this integration test had not been updated to track it.	2026-08-18 15:50:29 +02:00
jpmschweitzer	68bea0cceb	fix(tests): correct two independent AttributeErrors in test_task_executor 1. `executor.max_concurrent` has never existed. Concurrency is capped by the module-level MAX_CONCURRENT_TASKS constant via asyncio.Semaphore( MAX_CONCURRENT_TASKS) in TaskExecutor.__init__ — confirmed with git log -p across this file's whole history (three commits), the name has always been the module constant, never an instance attribute. test_executor_initialization now asserts MAX_CONCURRENT_TASKS == 5 and the semaphore's initial count, instead of a name the class never had. test_concurrent_task_limit asserted `mock_execute.call_count <= executor.max_concurrent`, which — separately from the AttributeError — was asserting the wrong observable: process_minute() awaits the full batch via asyncio.gather before returning, so by the time the assertion runs all 10 scheduled tasks have executed; the semaphore bounds how many run concurrently mid-flight, not the eventual call_count. Reworded to assert all scheduled tasks still run (call_count == len(tasks)); a concurrency- in-flight assertion would need a task that can be observed mid-execution, which the AsyncMock stand-in does not provide. 2. _run_executor (src/tasks/executor.py) loads the executor module with the __import__ builtin directly (`__import__(module_path, fromlist= ['execute'])`), not importlib.import_module — this repo's own CLAUDE.md documents it as "the thing that will mislead you" about this module. importlib is never imported there, so patch('src.tasks.executor.importlib. import_module') failed at patch setup, before the three test_execute_task_* bodies ran at all. Switched to patch('builtins. __import__', side_effect=...) with a routing function that falls through to the real import for anything other than the target module — verified the call-recording shape empirically first (call('name', fromlist=[...])). Source is unchanged in both cases; both are test-only defects present since this file's initial commit.	2026-08-18 15:49:37 +02:00
jpmschweitzer	874f9f9711	fix(tests): repair three mock-wiring bugs in doc_sync_executor tests test_executor_successful_sync_entire_repo: _get_git_commit is patched separately and never calls the real _run_command, so it does not consume a slot in mock_run.side_effect. The list reserved one anyway (labelled "git rev-parse HEAD"), which shifted "M README.md\n" one call late — git status --porcelain saw "" (no changes) instead, so execute() took the no-changes branch and the test asserted "Successfully synced" against "already up to date". Removed the phantom slot. Also gave the iterdir() mock items real string .name attributes: MagicMock(name="X") sets the mock's repr, not the .name attribute read by execute()'s `item.name != '.git'` check, so every item was being treated as non-.git regardless of the intended value. test_executor_sync_specific_paths, test_executor_handles_no_changes: mock_upstream_dir/mock_gitea_dir were built but never wired to mock_path.return_value, so `work_dir = Path(...)` and `upstream_dir = work_dir / "upstream"` resolved to a different, unconfigured auto-generated MagicMock. `source.name` on that mock is itself a MagicMock, not a string, so `', '.join(copied_paths)` raised TypeError. Wired mock_path.return_value to a work_dir mock whose __truediv__ yields the intended upstream/gitea mocks, matching the pattern the first test already used correctly. All three are test-side: doc_sync_executor.py is unchanged. Confirmed via git log -p that this file and its test have exactly one commit in this repo's history (the initial extraction), so there is no prior passing version to regress from — these tests appear to have never passed.	2026-08-18 15:49:20 +02:00
jpmschweitzer	5ccfb83f2f	fix(tests): correct two independent test-api defects found while auditing T-55 1. get_task_executor mocking: same dependency_overrides fix as the previous commit, applied to this file's remaining patch('src.main.get_task_executor') call sites (test_valid_api_key_allows_access, test_create_task_with_valid_data, test_trigger_task_endpoint, test_stats_endpoint_returns_metrics, test_executions_endpoint_returns_history, test_executions_filter_by_task_name). 2. test_create_task_missing_fields_returns_400 asserted a status this endpoint cannot return. `task: TaskCreate` in src/main.py is a plain Pydantic request body with no custom validation for missing fields — FastAPI's own dependency-resolution layer rejects the request before create_task's body runs, and that layer always answers 422, not 400. There is no code path in this repo, at any point in its git history, that produces 400 for this request. Renamed to test_create_task_missing_fields_returns_422 and the assertion updated to match; source unchanged. Kept together in one commit because both live in the same small file and were found in the same pass, rather than risk a manual hunk split on tightly interleaved diff context.	2026-08-18 15:49:08 +02:00
jpmschweitzer	e39234b436	fix(tests): use dependency_overrides for get_task_executor, not patch() FastAPI resolves Depends(get_task_executor) against the function object it captured when each route was decorated, at import time. patch('src.main. get_task_executor') therefore never reaches an already-registered route — confirmed empirically with a minimal FastAPI app before touching this repo's tests. Every one of these tests fell through to the real dependency, which called the real psycopg2.connect() and failed on postgres-shared's DNS (a Docker-network name unreachable from a host process either way, so this reproduced identically with and without host networking). app.dependency_overrides is FastAPI's own supported mechanism for this, and was already used correctly elsewhere in this suite (test_task_delete.py). Added a shared override_task_executor fixture in conftest.py and switched every get_task_executor patch() call site in test_api_comprehensive.py to it. Source (src/main.py) is unchanged — Depends() is the idiomatic, correct pattern; the tests were using an ineffective substitute for it.	2026-08-18 15:48:29 +02:00
jpmschweitzerandClaude	f9e1409898	build(setup): prove the venv actually works instead of trusting pip's exit code `make setup` created the venv and ran pip install, then exited 0 whether or not the result was usable — exactly D-24's failure shape, and the one this repo hit hardest: on 2026-08-09 there was no venv at all here while CLAUDE.md documented .venv/bin/python -m pytest as the way to run tests, and nothing in setup would have caught that state before a person did. Add a `pytest tests/ --collect-only -q` step at the end. It imports every test module and everything each one pulls in from src/, without running a single test, and fails the target on a broken interpreter or a broken dependency graph alike. Verified directly: - absent interpreter -> exit 127, target halts - apscheduler (declared, imported by src/main.py) uninstalled -> collect-only exits 4 with ModuleNotFoundError, target halts - make setup afterwards reinstalls it and collect-only exits 0, 189 tests collected, same count as before the mutation - idempotent: a warm rerun on an already-correct venv changes nothing and collects the same 189 tests, just faster (7s vs 30s cold) T-47 (workspace). Co-Authored-By: Claude <noreply@anthropic.com>	2026-08-17 12:11:52 +02:00