Docs/Tasks.md

@@ -7,127 +7,3 @@ Reference: `Docs/Refactoring.md` for full analysis of each issue.
 ---
 
 ## Completed Issues
-
-
-1. Move runtime application state out of `app.state`
-   - Goal: Remove process-local mutable business state from FastAPI application state and centralise it in a cluster-safe, testable runtime state abstraction.
-   - Issue: `app.state` is currently used to store runtime values such as `setup_complete_cached`, `server_status`, `pending_recovery`, and `last_activation`, which are process-specific and leak operational state into the framework layer.
-   - Propose: Introduce a dedicated runtime state manager or cache service that owns these values and exposes them through explicit dependencies. Keep `app.state` limited to shared immutable resources and infrastructure handles.
-   - Test: Verify the backend no longer stores business state in `app.state` and that runtime state is provided by the new abstraction only.
-   - Status: completed
-
-2. Eliminate direct `app.state` access from routers
-   - Goal: Keep routers strictly in the HTTP translation layer by using FastAPI `Depends()` and injected services only.
-   - Issue: Several routers still read or mutate `app.state` directly, including `setup.py` and `config.py`, which breaks the intended separation of concerns and makes handlers harder to test.
-   - Propose: Refactor routers to consume explicit dependencies for required values and actions, and move all direct state mutations into services or dedicated state managers.
-   - Test: Ensure all router handlers compile without requiring direct `app.state` access and existing endpoint tests still pass.
-   - Status: completed
-
-3. Replace process-local session cache with a pluggable abstraction
-   - Goal: Make session validation consistent across multi-worker deployments by replacing the module-level in-memory cache with an injectable cache backend.
-   - Issue: `_session_cache` in `app/dependencies.py` is a process-local dict, so logout invalidation and session revocation only work within a single process.
-   - Propose: Define a cache interface and provide a default in-memory implementation, with the option to swap in shared cache storage (Redis, Memcached) for clustered production deployments.
-   - Test: Add unit tests for the cache abstraction and verify logout/invalidation behaves correctly through the configured cache implementation.
-   - Status: completed
-
-4. Harden SQLite connection configuration and lifecycle
-   - Goal: Make application database access robust under concurrent requests and background task load.
-   - Issue: `get_db` opens a fresh `aiosqlite` connection per request without explicit database pragmas such as `busy_timeout` or WAL mode.
-   - Propose: Configure SQLite connections in `open_db` with safe defaults (e.g. WAL, busy timeout, journal mode) and consider a centralized request-scoped access wrapper to keep connection setup consistent.
-   - Test: Confirm `open_db` applies the expected pragmas and add simulated concurrency tests that surface lock / timeout failures.
-   - Status: completed
-
-5. Separate startup settings from runtime configuration mutation
-   - Goal: Keep startup configuration immutable after bootstrap and handle runtime overrides through a dedicated manager.
-   - Issue: `POST /api/setup` mutates `app.state.settings`, and startup logic also replaces `app.state.settings` with persisted runtime overrides, mixing immutable startup settings with mutable runtime config.
-   - Propose: Introduce a runtime settings manager or configuration service that resolves effective settings from persisted overrides without mutating the startup settings object on `app.state`.
-   - Test: Validate the setup flow persists runtime settings in the database and that runtime settings are resolved through the new manager instead of direct `app.state.settings` mutation.
-   - Status: completed
-
-6. Introduce explicit service and repository abstractions for swapability and testing
-   - Goal: Reduce tight coupling and improve replacement/testing of core backend components.
-   - Issue: Routers and higher-level services currently import concrete service modules directly, which prevents clean substitution and dependency override testing.
-   - Propose: Define protocols or abstract base classes for major services and repositories, then wire concrete implementations through FastAPI dependency providers.
-   - Test: Add tests that override a service dependency with a fake implementation and verify the router behavior remains correct.
-   - Status: completed
-   - Completed: Added service/repository protocols, injected auth/jail services via FastAPI dependencies, and added router tests for dependency overrides.
-
-7. Move operational orchestration out of routers and into service/task layer
-   - Goal: Keep routers thin and move operational control flow into service or task components.
-   - Issue: `app/routers/config.py` contains operational orchestration such as activation crash tracking, pending recovery state updates, and forced health probes, which belongs in service/task code not the HTTP layer.
-   - Propose: Refactor jail activation/deactivation/recovery coordination into services or task managers that manage state updates and health probe triggers on behalf of the router.
-   - Test: Confirm router tests only cover HTTP translation while unit tests for the new service/task components cover the orchestration logic.
-   - Status: completed
-
-8. Decouple periodic background jobs from FastAPI application state
-   - Goal: Make scheduled task runners explicit and testable by removing direct `app.state` dependency from background task code.
-   - Issue: `history_sync`, `geo_re_resolve`, `geo_cache_flush`, and `blocklist_import` read shared resources directly from `app.state` and register jobs against `app.state.scheduler`, which couples task implementation to FastAPI internals.
-   - Propose: Introduce a scheduler/task bootstrap abstraction that accepts injected settings, database providers, HTTP sessions, and scheduler handles, and move task resource access out of the `app.state` internals.
-   - Test: Add unit tests for task registration and execution with fake resource providers and a mock scheduler, without needing a real FastAPI instance.
-   - Status: completed
-   - Completed: Refactored background task modules to accept explicit scheduler job resources, removed direct `app.state` dependency from task execution callbacks, and updated unit tests to verify registration with injected settings, http session, and runtime state.
-
-9. Remove brittle scheduler bootstrap logic from blocklist job registration
-   - Goal: Ensure job registration is deterministic and compatible with both synchronous and asynchronous startup contexts.
-   - Issue: `blocklist_import.register` uses `asyncio.get_event_loop().run_until_complete` then falls back to `asyncio.ensure_future`, which is fragile and can fail if the application is already running inside an event loop.
-   - Propose: Refactor blocklist job registration to use a pure async bootstrap flow invoked from the lifespan handler, or resolve schedule configuration before starting the scheduler instead of forcing synchronous loop execution.
-   - Test: Add tests validating blocklist job registration in both non-running and running event loop contexts, and verify the registration path does not raise loop state errors.
-   - Status: completed
-
-10. Standardize async offloading and thread-backed sync work behind a shared executor abstraction
-   - Goal: Remove scattered `asyncio.get_event_loop().run_in_executor` / `asyncio.ensure_future` patterns and give the backend a single, consistent way to run blocking file, CPU-bound, or compatibility work.
-   - Issue: Multiple services (`raw_config_io_service`, `log_service`, `jail_config_service`, `setup_service`, and others) manually call `asyncio.get_event_loop()` or `run_in_executor`, creating inconsistent async control flow and making event-loop compatibility harder to maintain.
-   - Propose: Introduce a shared async offload abstraction or utility that uses `asyncio.to_thread` / dedicated thread pool executors, and refactor blocking I/O and CPU-bound helpers to consume that shared provider.
-   - Test: Verify existing async service tests still pass after refactoring and add coverage for the new offload utility to ensure blocking work is properly delegated without event-loop blocking.
-   - Status: completed
-
-11. Eliminate duplicate setup persistence across bootstrap and runtime databases
-   - Goal: Simplify setup persistence by establishing a single source of truth for runtime configuration data and reduce failure surface in first-run setup.
-   - Issue: `setup_service.run_setup` writes the same master password and runtime settings to both the bootstrap configuration database and the runtime database, making setup logic brittle and difficult to reason about.
-   - Propose: Consolidate setup persistence so initial configuration is stored in one clearly defined location, with bootstrap metadata in the startup DB and runtime configuration pulled from the single persisted source.
-   - Test: Add tests that confirm setup writes each value exactly once and that startup uses a single persisted store to resolve runtime settings, removing duplicate write paths.
-   - Status: completed
-
-12. Replace custom fail2ban socket protocol duplication with a dedicated adapter over the vendored client
-   - Goal: Reduce maintenance risk by aligning fail2ban integration with the bundled `fail2ban-master` codebase and isolating protocol implementation behind a swapable adapter.
-   - Issue: `app.utils.fail2ban_client` reimplements low-level socket framing, command encoding, and protocol parsing rather than using the vendored fail2ban client classes, creating duplicated protocol logic and an unclear source of truth.
-   - Propose: Introduce a fail2ban adapter interface and either wrap the vendored `fail2ban-client` implementation or refactor the custom client so it is the single canonical integration point. Ensure all services depend on the adapter abstraction rather than raw socket details.
-   - Test: Add adapter-level unit tests and service tests that can swap in a fake fail2ban adapter, proving the backend no longer couples business logic directly to low-level socket protocol code.
-   - Status: completed
-
-13. Introduce explicit schema migration/versioning for the runtime database
-   - Goal: Allow BanGUI to evolve its application database schema safely across releases and prevent startup failures caused by schema drift.
-   - Issue: `app/db.py` only creates missing tables on startup and has no schema version marker or migration strategy. This leaves upgrades brittle and makes it difficult to evolve the database without manual intervention.
-   - Propose: Add a lightweight migration system or schema version table, define incremental migration scripts, and run upgrades during startup before the application begins serving requests.
-   - Test: Add tests that simulate an older schema version and verify startup migrates it to the latest schema, and assert that `init_db`/migration code leaves the database in the expected current state.
-   - Status: completed
-
-14. Split the monolithic config router into focused sub-routers
-   - Goal: Improve maintainability, reduce cognitive load, and make HTTP routing responsibilities easier to isolate and test.
-   - Issue: `app/routers/config.py` and `app/routers/file_config.py` each contain many unrelated endpoints with mixed concerns, making the HTTP layer large, hard to navigate, and harder to evolve safely.
-   - Propose: Refactor configuration endpoints into smaller routers or feature-specific modules such as `jail_config`, `filter_config`, `action_config`, and `raw_config`. Keep each router focused on one vertical slice of functionality and delegate business logic to a single service.
-   - Test: Verify the refactor preserves endpoint behavior and that router tests are split along the new modules, ensuring each HTTP layer unit test covers a narrow, well-defined feature.
-   - Status: completed
-
-15. Centralize fail2ban metadata resolution and avoid repeated socket discovery
-   - Goal: Reduce coupling between history APIs and fail2ban socket metadata discovery, improving performance and reliability.
-   - Issue: `app/utils/fail2ban_db_utils.get_fail2ban_db_path` queries fail2ban for the database path on every history request. This ties read-only history endpoints to socket availability and duplicates discovery logic across the codebase.
-   - Propose: Introduce a dedicated fail2ban metadata service that resolves and caches runtime metadata such as the fail2ban DB path, refreshing it only when needed instead of every request.
-   - Test: Add tests that confirm history requests can reuse cached metadata, and that the metadata service falls back cleanly when the socket is temporarily unavailable after initial resolution.
-   - Status: completed
-
-16. Introduce an explicit application lifecycle context to replace raw `app.state` access
-   - Goal: Make shared infrastructure handles and runtime configuration available through a typed, injectable context instead of a loosely-typed `app.state` bag.
-   - Issue: The backend currently relies on raw `app.state` access in routers, tasks, and dependencies, which hides contract boundaries and allows arbitrary mutable state to leak into the framework layer.
-   - Propose: Define a dedicated `ApplicationContext` / runtime context object for shared resources such as `settings`, `http_session`, `scheduler`, and runtime caches. Provide it through explicit FastAPI dependencies and remove direct `request.app.state` reads outside a small bootstrap boundary.
-   - Test: Verify the new context can satisfy existing dependencies, and add a static check or test ensuring no backend module directly accesses `request.app.state` except through the new context provider.
-   - Status: completed
-
-17. Resolve runtime configuration before creating shared startup resources
-   - Goal: Ensure startup resources are initialized using the effective runtime settings, not stale bootstrap defaults.
-   - Issue: `startup_shared_resources` creates shared resources like `aiohttp.ClientSession` and geo cache initialization from the initial environment-loaded settings, then later applies persisted runtime overrides to `app.state.settings`, producing a fragile startup ordering.
-   - Propose: Split startup into phases that first resolve bootstrap and runtime persisted configuration, then construct shared resources and register scheduled jobs using those effective settings.
-   - Test: Add startup tests asserting that when persisted runtime settings differ from bootstrap settings, the final initialized resources are built from the resolved effective settings, not the original bootstrap values.
-
-
-

backend/app/dependencies.py

@@ -97,22 +97,31 @@ async def get_app_context(request: Request) -> ApplicationContext:
     return _build_app_context(request)
 
 
-async def get_db(app_context: Annotated[ApplicationContext, Depends(get_app_context)]) -> AsyncGenerator[aiosqlite.Connection, None]:
+async def get_settings(app_context: Annotated[ApplicationContext, Depends(get_app_context)]) -> Settings:
+    """Provide the effective application settings for the current request."""
+    return app_context.runtime_settings if app_context.runtime_settings is not None else app_context.settings
+
+
+async def get_db(
+    settings: Annotated[Settings, Depends(get_settings)],
+) -> AsyncGenerator[aiosqlite.Connection, None]:
     """Provide a request-scoped :class:`aiosqlite.Connection` for the current request.
 
     Opens a fresh connection for every request and closes it when the request
     is finished. This avoids contention and locking issues from a single shared
     SQLite connection across concurrent requests.
 
+    The database path is taken from the effective application settings so
+    runtime overrides stored during setup are respected.
+
     Args:
-        app_context: The injected shared application context.
+        settings: The effective application settings for the current request.
 
     Yields:
         An open :class:`aiosqlite.Connection` for the request.
     """
     from app.db import open_db  # noqa: PLC0415
 
-    settings = app_context.settings
     try:
         db = await open_db(settings.database_path)
     except Exception as exc:
@@ -128,11 +137,6 @@ async def get_db(app_context: Annotated[ApplicationContext, Depends(get_app_cont
         await db.close()
 
 
-async def get_settings(app_context: Annotated[ApplicationContext, Depends(get_app_context)]) -> Settings:
-    """Provide the effective application settings for the current request."""
-    return app_context.runtime_settings if app_context.runtime_settings is not None else app_context.settings
-
-
 async def get_http_session(app_context: Annotated[ApplicationContext, Depends(get_app_context)]) -> aiohttp.ClientSession:
     """Provide the shared HTTP client session from application context.
 

backend/tests/test_dependencies.py

@@ -1,7 +1,7 @@
 from __future__ import annotations
 
 from pathlib import Path
-from unittest.mock import MagicMock
+from unittest.mock import AsyncMock, MagicMock, patch
 
 import aiohttp
 import pytest
@@ -12,6 +12,7 @@ from app.config import Settings
 from app.dependencies import (
     ApplicationContext,
     get_app_context,
+    get_db,
     get_http_session,
     get_scheduler,
     get_settings,
@@ -64,6 +65,25 @@ async def test_app_context_dependency_exposes_shared_resources(test_settings: Se
     await session.close()
 
 
+@pytest.mark.asyncio
+async def test_get_db_uses_effective_runtime_database_path(test_settings: Settings) -> None:
+    """Database connections should use effective runtime settings when overridden."""
+    runtime_settings = test_settings.model_copy(update={"database_path": "/tmp/runtime.db"})
+
+    mock_connection = MagicMock()
+    mock_connection.close = AsyncMock()
+
+    with patch("app.db.open_db", new=AsyncMock(return_value=mock_connection)) as mock_open_db:
+        gen = get_db(settings=runtime_settings)
+        try:
+            connection = await gen.__anext__()
+            assert connection is mock_connection
+        finally:
+            await gen.aclose()
+
+    mock_open_db.assert_awaited_once_with("/tmp/runtime.db")
+
+
 def test_request_app_state_access_is_only_allowed_in_dependencies() -> None:
     app_root = Path(__file__).resolve().parents[1] / "app"
     bad_modules: list[str] = []