Remove compat shim indirection, simplify MCP tool registration

Replace the fragile pattern where service modules accessed get_settings,
boto3, and date through a compat re-export module (services.py) with
direct imports. Tests now patch at the point of use, which is the
standard Python mocking practice.

BackupService.__init__ no longer calls get_settings() when backup_dir
is provided explicitly, removing an unnecessary coupling that broke
tests.

Simplify mcp/server.py from 49 lines of individual function imports
to 8 module imports for tool registration. Tests import from the
source modules directly instead of through server.py re-exports.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Author: davidtorcivia

invoice_machine/mcp/server.py

@@ -2,56 +2,19 @@
 
 from __future__ import annotations
 
-from .analytics_tools import (  # noqa: F401
-    get_client_invoice_context,
-    get_client_lifetime_value,
-    get_revenue_summary,
-)
-from .client_tools import (  # noqa: F401
-    create_client,
-    delete_client,
-    get_client,
-    list_clients,
-    restore_client,
-    update_client,
+# Import tool modules to register @mcp.tool() decorated functions.
+# New tools added to any module are automatically registered.
+from . import (  # noqa: F401
+    analytics_tools,
+    client_tools,
+    document_tools,
+    email_tools,
+    invoice_tools,
+    profile_tools,
+    recurring_tools,
+    search_tools,
 )
 from .context import mcp
-from .document_tools import generate_pdf, list_trash  # noqa: F401
-from .email_tools import (  # noqa: F401
-    get_email_templates,
-    preview_invoice_email,
-    send_invoice_email,
-    test_smtp_connection,
-    update_email_templates,
-)
-from .invoice_tools import (  # noqa: F401
-    add_invoice_item,
-    create_invoice,
-    delete_invoice,
-    get_invoice,
-    list_invoices,
-    remove_invoice_item,
-    restore_invoice,
-    update_invoice,
-    update_invoice_item,
-)
-from .profile_tools import (  # noqa: F401
-    add_payment_method,
-    get_business_profile,
-    remove_payment_method,
-    update_business_profile,
-)
-from .recurring_tools import (  # noqa: F401
-    create_recurring_schedule,
-    delete_recurring_schedule,
-    get_recurring_schedule,
-    list_recurring_schedules,
-    pause_recurring_schedule,
-    resume_recurring_schedule,
-    trigger_recurring_schedule,
-    update_recurring_schedule,
-)
-from .search_tools import search  # noqa: F401
 
 
 def main():

invoice_machine/service/backups.py

@@ -8,25 +8,13 @@
 from datetime import UTC, datetime, timedelta
 from pathlib import Path
 
+import boto3
 from botocore.config import Config
 
+from invoice_machine.config import get_settings
 from invoice_machine.utils import utc_now
 
 
-def _compat_get_settings():
-    """Read settings through the compatibility module to keep legacy patches working."""
-    from invoice_machine import services as compat
-
-    return compat.get_settings()
-
-
-def _compat_boto3_client(*args, **kwargs):
-    """Create a boto3 client through the compatibility module for legacy patches."""
-    from invoice_machine import services as compat
-
-    return compat.boto3.client(*args, **kwargs)
-
-
 class BackupService:
     """Service for creating and managing backups."""
 
@@ -36,15 +24,16 @@ def __init__(
         retention_days: int = 30,
         s3_config: dict | None = None,
     ):
-        settings = _compat_get_settings()
-        self.backup_dir = backup_dir or settings.data_dir / "backups"
+        if backup_dir is None:
+            backup_dir = get_settings().data_dir / "backups"
+        self.backup_dir = backup_dir
         self.retention_days = retention_days
         self.s3_config = s3_config
         self.backup_dir.mkdir(parents=True, exist_ok=True)
 
     def create_backup(self, compress: bool = True) -> dict:
         """Create a backup of the database and optionally upload it to S3."""
-        settings = _compat_get_settings()
+        settings = get_settings()
         timestamp = utc_now()
         timestamp_str = timestamp.strftime("%Y%m%d_%H%M%S")
 
@@ -88,7 +77,7 @@ def _upload_to_s3(self, local_path: Path, filename: str):
         if not self.s3_config:
             raise ValueError("S3 configuration not provided")
 
-        s3_client = _compat_boto3_client(
+        s3_client = boto3.client(
             "s3",
             endpoint_url=self.s3_config.get("endpoint_url"),
             aws_access_key_id=self.s3_config.get("access_key_id"),
@@ -143,7 +132,7 @@ def _cleanup_s3_backups(self, cutoff: datetime) -> int:
         if not self.s3_config:
             return 0
 
-        s3_client = _compat_boto3_client(
+        s3_client = boto3.client(
             "s3",
             endpoint_url=self.s3_config.get("endpoint_url"),
             aws_access_key_id=self.s3_config.get("access_key_id"),
@@ -222,7 +211,7 @@ def get_backup_path(self, backup_filename: str, must_exist: bool = True) -> Path
 
     def restore_backup(self, backup_filename: str, validate: bool = True) -> dict:
         """Restore the database from a backup using atomic replacement."""
-        settings = _compat_get_settings()
+        settings = get_settings()
         backup_path = self.get_backup_path(backup_filename)
         if validate:
             self.validate_backup(backup_path)
@@ -259,7 +248,7 @@ def download_from_s3(self, filename: str) -> Path:
             raise ValueError("S3 is not configured")
 
         local_path = self._validate_backup_filename(filename)
-        s3_client = _compat_boto3_client(
+        s3_client = boto3.client(
             "s3",
             endpoint_url=self.s3_config.get("endpoint_url"),
             aws_access_key_id=self.s3_config.get("access_key_id"),
@@ -279,7 +268,7 @@ def list_s3_backups(self) -> list[dict]:
             return []
 
         try:
-            s3_client = _compat_boto3_client(
+            s3_client = boto3.client(
                 "s3",
                 endpoint_url=self.s3_config.get("endpoint_url"),
                 aws_access_key_id=self.s3_config.get("access_key_id"),

invoice_machine/service/recurring.py

@@ -16,13 +16,6 @@
 from invoice_machine.utils import utc_now
 
 
-def _today() -> date:
-    """Read today's date through the compatibility module to keep legacy patches working."""
-    from invoice_machine import services as compat
-
-    return compat.date.today()
-
-
 class RecurringService:
     """Service for managing recurring invoice schedules."""
 
@@ -51,7 +44,7 @@ async def create_schedule(
         )
 
         if next_invoice_date is None:
-            today = _today()
+            today = date.today()
             if frequency == "daily":
                 next_invoice_date = today + timedelta(days=1)
             elif frequency == "weekly":
@@ -149,7 +142,7 @@ async def update_schedule(
             and "next_invoice_date" not in kwargs
         ):
             kwargs["next_invoice_date"] = RecurringService.calculate_next_date(
-                _today(), new_frequency, new_schedule_day
+                date.today(), new_frequency, new_schedule_day
             )
 
         for key, value in kwargs.items():
@@ -219,7 +212,7 @@ async def process_due_schedules(session: AsyncSession) -> list[dict]:
         """Process all schedules due today or earlier and create invoices."""
         from invoice_machine.service.invoices import InvoiceService
 
-        today = _today()
+        today = date.today()
         result = await session.execute(
             select(RecurringSchedule).where(
                 RecurringSchedule.is_active == 1,
@@ -278,7 +271,7 @@ async def trigger_schedule(session: AsyncSession, schedule_id: int) -> dict:
         if not schedule:
             return {"success": False, "error": "Schedule not found"}
 
-        today = _today()
+        today = date.today()
         try:
             line_items = json.loads(schedule.line_items) if schedule.line_items else []
             invoice = await InvoiceService.create_invoice(

invoice_machine/services.py

@@ -1,10 +1,5 @@
 """Compatibility exports for the service layer."""
 
-from datetime import date  # noqa: F401 — accessed via compat.date by service modules
-
-import boto3  # noqa: F401 — accessed via compat.boto3 by BackupService
-
-from invoice_machine.config import get_settings  # noqa: F401 — accessed via compat.get_settings
 from invoice_machine.service.backups import BackupService, get_backup_service
 from invoice_machine.service.clients import ClientService
 from invoice_machine.service.common import (

tests/test_backup.py

@@ -73,7 +73,7 @@ def test_create_backup_compressed(self, mock_db_file):
             backup_dir = Path(tmpdir) / "backups"
             service = BackupService(backup_dir=backup_dir)
 
-            with patch("invoice_machine.services.get_settings") as mock_settings:
+            with patch("invoice_machine.service.backups.get_settings") as mock_settings:
                 mock_settings.return_value.data_dir = mock_db_file.parent
 
                 result = service.create_backup(compress=True)
@@ -95,7 +95,7 @@ def test_create_backup_uncompressed(self, mock_db_file):
             backup_dir = Path(tmpdir) / "backups"
             service = BackupService(backup_dir=backup_dir)
 
-            with patch("invoice_machine.services.get_settings") as mock_settings:
+            with patch("invoice_machine.service.backups.get_settings") as mock_settings:
                 mock_settings.return_value.data_dir = mock_db_file.parent
 
                 result = service.create_backup(compress=False)
@@ -115,7 +115,7 @@ def test_create_backup_db_not_found(self):
             backup_dir = Path(tmpdir) / "backups"
             service = BackupService(backup_dir=backup_dir)
 
-            with patch("invoice_machine.services.get_settings") as mock_settings:
+            with patch("invoice_machine.service.backups.get_settings") as mock_settings:
                 mock_settings.return_value.data_dir = Path("/nonexistent")
 
                 with pytest.raises(FileNotFoundError, match="Database not found"):
@@ -134,7 +134,7 @@ def test_create_backup_with_s3_upload(self, mock_db_file):
             }
             service = BackupService(backup_dir=backup_dir, s3_config=s3_config)
 
-            with patch("invoice_machine.services.get_settings") as mock_settings:
+            with patch("invoice_machine.service.backups.get_settings") as mock_settings:
                 mock_settings.return_value.data_dir = mock_db_file.parent
 
                 with patch.object(service, "_upload_to_s3") as mock_upload:
@@ -150,7 +150,7 @@ def test_create_backup_s3_upload_failure(self, mock_db_file):
             s3_config = {"enabled": True, "bucket": "test-bucket"}
             service = BackupService(backup_dir=backup_dir, s3_config=s3_config)
 
-            with patch("invoice_machine.services.get_settings") as mock_settings:
+            with patch("invoice_machine.service.backups.get_settings") as mock_settings:
                 mock_settings.return_value.data_dir = mock_db_file.parent
 
                 with patch.object(
@@ -169,7 +169,7 @@ def test_create_backup_timestamp_in_filename(self, mock_db_file):
             backup_dir = Path(tmpdir) / "backups"
             service = BackupService(backup_dir=backup_dir)
 
-            with patch("invoice_machine.services.get_settings") as mock_settings:
+            with patch("invoice_machine.service.backups.get_settings") as mock_settings:
                 mock_settings.return_value.data_dir = mock_db_file.parent
 
                 result = service.create_backup()
@@ -293,7 +293,7 @@ def test_upload_to_s3_success(self):
             }
             service = BackupService(backup_dir=backup_dir, s3_config=s3_config)
 
-            with patch("invoice_machine.services.boto3.client") as mock_boto:
+            with patch("invoice_machine.service.backups.boto3.client") as mock_boto:
                 mock_client = MagicMock()
                 mock_boto.return_value = mock_client
 
@@ -321,7 +321,7 @@ def test_upload_to_s3_default_prefix(self):
             }
             service = BackupService(backup_dir=backup_dir, s3_config=s3_config)
 
-            with patch("invoice_machine.services.boto3.client") as mock_boto:
+            with patch("invoice_machine.service.backups.boto3.client") as mock_boto:
                 mock_client = MagicMock()
                 mock_boto.return_value = mock_client
 
@@ -406,7 +406,7 @@ def test_restore_backup_success(self):
 
             service = BackupService(backup_dir=backup_dir)
 
-            with patch("invoice_machine.services.get_settings") as mock_settings:
+            with patch("invoice_machine.service.backups.get_settings") as mock_settings:
                 mock_settings.return_value.data_dir = data_dir
 
                 result = service.restore_backup(
@@ -440,7 +440,7 @@ def test_restore_compressed_backup(self):
 
             service = BackupService(backup_dir=backup_dir)
 
-            with patch("invoice_machine.services.get_settings") as mock_settings:
+            with patch("invoice_machine.service.backups.get_settings") as mock_settings:
                 mock_settings.return_value.data_dir = data_dir
 
                 result = service.restore_backup(
@@ -481,7 +481,7 @@ def test_restore_creates_pre_restore_backup(self):
 
             service = BackupService(backup_dir=backup_dir)
 
-            with patch("invoice_machine.services.get_settings") as mock_settings:
+            with patch("invoice_machine.service.backups.get_settings") as mock_settings:
                 mock_settings.return_value.data_dir = data_dir
 
                 result = service.restore_backup(
@@ -510,7 +510,7 @@ def test_restore_backup_cleans_up_temporary_restore_file(self):
 
             service = BackupService(backup_dir=backup_dir)
 
-            with patch("invoice_machine.services.get_settings") as mock_settings:
+            with patch("invoice_machine.service.backups.get_settings") as mock_settings:
                 mock_settings.return_value.data_dir = data_dir
 
                 service.restore_backup(