fix(db): follow-up improvements for database configuration

AGENTS.md

@@ -103,6 +103,16 @@ Common migration commands:
 - Type errors → Add `# type: ignore[error-code]` with specific code
 - Import warnings → Add `# noqa: PLC0415` for intentional imports inside functions
 
+## Implementation Patterns (Critical)
+
+**Exception Handling & Variable Scoping:**
+
+- **Variables assigned in `try` blocks**: If you use a variable after `except`/`finally`, ensure it's initialized before the `try` or guard its usage. Common bug: `UnboundLocalError` when exceptions occur before assignment.
+- **Code after exception handlers**: Don't access variables that may not exist if an exception occurred. Move variable-dependent operations into the `try` block (after successful assignment) or guard with existence checks.
+- **Cleanup in `finally` blocks**: Only access variables guaranteed to exist regardless of exception path.
+
+**Environment variables**: When adding new env vars, update `.env.example` if it exists (`test -f .env.example`).
+
 ## Where To Read More
 
 **Essential References:**

backend/cli/db.py

@@ -119,7 +119,10 @@ def dump(out: Path = typer.Option(..., "--out", help="Output SQL file path")) ->
 
 
 @db_app.command(name="restore")
-def restore(input_file: Path = typer.Option(..., "--in", help="Input SQL file path")) -> None:
+def restore(
+    input_file: Path = typer.Option(..., "--in", help="Input SQL file path"),
+    yes: bool = typer.Option(False, "--yes", help="Skip confirmation"),
+) -> None:
     """Restore database from SQL file (WARNING: overwrites existing database)."""
     if not input_file.exists():
         console.print(f"[red]✗[/red] Input file not found: {input_file}")
@@ -128,8 +131,8 @@ def restore(input_file: Path = typer.Option(..., "--in", help="Input SQL file pa
     settings = backend.config.get_settings()
     db_path = settings.database_path
 
-    # Confirm overwrite
-    if db_path.exists():
+    # Confirm overwrite only if DB exists and --yes not provided
+    if not yes and db_path.exists():
         console.print(f"[yellow]⚠[/yellow]  This will overwrite the existing database at {db_path}")
         if not Confirm.ask("Continue?", default=False):
             console.print("[yellow]Restore cancelled[/yellow]")

backend/config.py

@@ -200,9 +200,9 @@ def ensure_app_dirs(self) -> None:
         """
         self.app_data_dir.mkdir(parents=True, exist_ok=True)
         (self.app_data_dir / "data").mkdir(parents=True, exist_ok=True)
-        (self.app_data_dir / "logs").mkdir(parents=True, exist_ok=True)
-        (self.app_data_dir / "cache").mkdir(parents=True, exist_ok=True)
-        (self.app_data_dir / "cache" / "attachments").mkdir(parents=True, exist_ok=True)
+        self.logs_path.mkdir(parents=True, exist_ok=True)
+        self.attachment_cache_path.mkdir(parents=True, exist_ok=True)
+        self.vector_store_path.mkdir(parents=True, exist_ok=True)
 
         database_path = self.database_path
         if str(database_path) != ":memory:":

backend/database.py

@@ -11,10 +11,11 @@
 import sqlite3
 from collections.abc import AsyncGenerator
 from pathlib import Path
+from typing import Any
 
 import alembic.command
 import alembic.config
-from sqlalchemy import text
+from sqlalchemy import event, text
 from sqlalchemy.ext.asyncio import AsyncEngine, AsyncSession, async_sessionmaker, create_async_engine
 from sqlalchemy.orm import declarative_base
 
@@ -32,6 +33,22 @@
 async_session_maker: async_sessionmaker[AsyncSession] | None = None
 
 
+def _enable_sqlite_foreign_keys(engine: AsyncEngine) -> None:
+    """Enable SQLite foreign keys for all connections via engine event.
+
+    This ensures foreign keys are enforced at the connection level,
+    not just at the session level.
+    """
+    if engine.url.get_backend_name() == "sqlite":
+
+        @event.listens_for(engine.sync_engine, "connect")
+        def set_sqlite_pragma(dbapi_conn: sqlite3.Connection, _connection_record: Any) -> None:
+            """Set PRAGMA foreign_keys=ON for SQLite connections."""
+            cursor = dbapi_conn.cursor()
+            cursor.execute("PRAGMA foreign_keys=ON")
+            cursor.close()
+
+
 def init_db() -> None:
     """Initialize database engine and sessionmaker.
 
@@ -53,6 +70,9 @@ def init_db() -> None:
     # Create async engine
     engine = create_async_engine(database_url, echo=settings.debug)
 
+    # Enable SQLite foreign keys at engine level
+    _enable_sqlite_foreign_keys(engine)
+
     # Create sessionmaker
     async_session_maker = async_sessionmaker(engine, class_=AsyncSession, expire_on_commit=False)
 
@@ -81,6 +101,9 @@ async def init_db_async() -> None:
     # Create async engine
     engine = create_async_engine(database_url, echo=settings.debug)
 
+    # Enable SQLite foreign keys at engine level
+    _enable_sqlite_foreign_keys(engine)
+
     # Create sessionmaker
     async_session_maker = async_sessionmaker(engine, class_=AsyncSession, expire_on_commit=False)
 

docs/02-implementation/pr-specs/PR-017-db-config-followups.md

@@ -2,7 +2,7 @@
 
 **Status:** Spec Only  
 **Depends on:** PR-001  
-**Last Reviewed:** 2025-12-30
+**Last Reviewed:** 2026-01-03
 
 ## Goal
 
@@ -35,12 +35,13 @@ SQLite FK enforcement, and docs alignment.
 
 ### In
 
-- Ensure `ensure_app_dirs()` creates the vector store directory (`data/chroma`).
-- Keep confirmation when `tgenie db restore` would overwrite an existing DB,
-  and add a `--yes` flag to skip the prompt for automation.
-- Enforce SQLite foreign keys on every SQLAlchemy connection (not just sessions).
+- Ensure `ensure_app_dirs()` creates the vector store directory (`data/chroma`) via
+  the canonical settings path helper.
+- Keep confirmation when `tgenie db restore` would overwrite an existing DB, and add
+  a `--yes` flag to skip the prompt for automation (consistent with `db reset`).
+- Enforce SQLite foreign keys on every SQLAlchemy engine connection (not just sessions).
 - Update `docs/TROUBLESHOOTING.md` to remove PR-001 "planned" wording and align
-  default DB path guidance with current settings.
+  default DB path guidance with current settings and environment overrides.
 
 ### Out
 
@@ -49,10 +50,10 @@ SQLite FK enforcement, and docs alignment.
 
 ## Mini-Specs
 
-- Add vector store directory creation to settings dir bootstrap.
-- Add a restore confirmation bypass flag for overwrite prompts.
-- Add engine-level SQLite FK enforcement hook.
-- Align troubleshooting docs with actual DB/config behavior.
+- Add vector store directory creation to settings dir bootstrap using `vector_store_path`.
+- Add a restore confirmation bypass flag (`--yes`) for overwrite prompts.
+- Add engine-level SQLite FK enforcement hook on each new DBAPI connection.
+- Align troubleshooting docs with actual DB/config behavior and default paths.
 
 ## User Stories
 
@@ -69,11 +70,17 @@ SQLite FK enforcement, and docs alignment.
 
 ### Architecture
 
-- In `Settings.ensure_app_dirs()`, create `data/chroma` using the same root as
-  `database_path`.
+- In `Settings.ensure_app_dirs()`, create `data/chroma` using
+  `Settings.vector_store_path` (same root as `database_path`).
+- Add `--yes` to `tgenie db restore` (in `backend/cli/db.py`) and gate the
+  confirmation prompt on `not yes` and `db_path.exists()`.
 - For SQLite, attach a SQLAlchemy engine `connect` event that runs
-  `PRAGMA foreign_keys=ON` on every new DBAPI connection (sync + async engines).
+  `PRAGMA foreign_keys=ON` on every new DBAPI connection (use
+  `event.listens_for(engine.sync_engine, "connect")` for async engines).
 - Keep `get_db()` PRAGMA as a secondary guard.
+- In `docs/TROUBLESHOOTING.md`, update the DB section to reflect the default
+  path `~/.taskgenie/data/taskgenie.db` and mention `TASKGENIE_DATA_DIR` or
+  `DATABASE_URL` overrides (no cwd-relative defaults).
 
 ### Data Model / Migrations
 
@@ -101,7 +108,7 @@ SQLite FK enforcement, and docs alignment.
 ### AC1: App Dir Completeness
 
 **Success Criteria:**
-- [ ] `ensure_app_dirs()` creates `data/chroma` under the resolved app data dir.
+- [ ] `ensure_app_dirs()` creates `data/chroma` using `Settings.vector_store_path`.
 - [ ] No directories are created at import time.
 
 ### AC2: Restore Confirmation
@@ -118,15 +125,17 @@ SQLite FK enforcement, and docs alignment.
 ### AC4: Docs Alignment
 
 **Success Criteria:**
-- [ ] `docs/TROUBLESHOOTING.md` reflects current DB wiring and default paths.
+- [ ] `docs/TROUBLESHOOTING.md` reflects current DB wiring, default paths, and
+  `TASKGENIE_DATA_DIR`/`DATABASE_URL` override behavior.
 
 ## Test Plan
 
 ### Automated
 
-- Unit: `ensure_app_dirs()` creates `data/chroma`.
+- Unit: `ensure_app_dirs()` creates `data/chroma` via `vector_store_path`.
 - CLI: `tgenie db restore` prompts only when the DB exists; `--yes` skips.
-- Integration: engine-level FK PRAGMA is applied for a fresh connection.
+- Integration: engine-level FK PRAGMA is applied for a fresh connection
+  (verify `PRAGMA foreign_keys` returns `1` on a new connection).
 
 ### Manual
 

docs/TROUBLESHOOTING.md

@@ -24,12 +24,12 @@
 
 ## Database file/path issues
 
-Database wiring (migrations, tables, dump/restore) is planned in PR-001. Today, `DATABASE_URL` is reserved for that work.
+The default database path is `~/.taskgenie/data/taskgenie.db` (derived from `TASKGENIE_DATA_DIR`).
 
-The default is `DATABASE_URL=sqlite+aiosqlite:///./data/taskgenie.db`, which is relative to your current working directory.
-
-- Run from repo root, or change `DATABASE_URL` to an absolute path.
-- Ensure the `data/` directory exists and is writable.
+- Override the data directory: set `TASKGENIE_DATA_DIR`
+- Override the database URL: set `DATABASE_URL` (e.g., `sqlite+aiosqlite:///path/to/db`)
+- Ensure the data directory exists and is writable
+- Run migrations: `tgenie db upgrade`
 
 ## `NOTIFICATION_SCHEDULE` parsing errors
 

tests/cli/test_db.py

@@ -420,6 +420,27 @@ def test_db_restore_confirmation_accepted(tmp_path: Path, monkeypatch: pytest.Mo
     assert db_path.exists()
 
 
+def test_db_restore_with_yes_flag(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
+    """Test that db restore works with --yes flag."""
+    db_path = tmp_path / "test.db"
+    monkeypatch.setenv("DATABASE_URL", f"sqlite+aiosqlite:///{db_path}")
+    config.get_settings.cache_clear()
+
+    # Create DB file and backup file
+    db_path.parent.mkdir(parents=True, exist_ok=True)
+    db_path.write_text("old data")
+    backup_file = tmp_path / "backup.sql"
+    backup_file.write_text("CREATE TABLE test (id INTEGER);")
+
+    runner = CliRunner()
+    # Use --yes flag, no input needed
+    result = runner.invoke(db_app, ["restore", "--in", str(backup_file), "--yes"])
+    assert result.exit_code == 0
+    assert "restored" in result.stdout.lower() or "✓" in result.stdout
+    # Verify old DB was deleted and new one created
+    assert db_path.exists()
+
+
 def test_db_restore_error(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
     """Test restore command error handling."""
     db_path = tmp_path / "test.db"

tests/test_config.py

@@ -95,6 +95,7 @@ def test_app_data_dir_creation(tmp_path: Path, monkeypatch: pytest.MonkeyPatch)
     assert (settings.app_data_dir / "data").exists()
     assert (settings.app_data_dir / "logs").exists()
     assert (settings.app_data_dir / "cache" / "attachments").exists()
+    assert settings.vector_store_path.exists()
 
 
 def test_database_url_resolved_default() -> None:

tests/test_database.py

@@ -201,6 +201,21 @@ async def test_get_db_foreign_keys_enabled(temp_settings: None) -> None:
     await close_db()
 
 
+@pytest.mark.asyncio
+async def test_engine_level_foreign_keys_enabled(temp_settings: None) -> None:
+    """Test that foreign keys are enabled at engine connection level (not just session level)."""
+    await init_db_async()
+    assert database.engine is not None
+
+    # Open a raw connection via engine.connect() (async)
+    async with database.engine.connect() as conn:
+        result = await conn.execute(text("PRAGMA foreign_keys"))
+        enabled = result.scalar()
+        assert enabled == 1, "Foreign keys should be enabled at engine connection level"
+
+    await close_db()
+
+
 def test_run_migrations_if_needed_memory_db(monkeypatch: pytest.MonkeyPatch) -> None:
     """Test _run_migrations_if_needed with :memory: database (covers lines 102-103)."""
     monkeypatch.setenv("DATABASE_URL", "sqlite+aiosqlite:///:memory:")