feat(db): implement P0 database TODOs - issue deps & audit logging (#216)

* feat(db): implement P0 database TODOs - issue deps & audit logging (#207)

Completes high-priority database integration items:

Issue Dependency Parsing:
- Add depends_on TEXT column to issues table schema
- Parse depends_on JSON field in get_issues_with_tasks()
- Handle NULL, invalid JSON, and non-list values gracefully
- Add depends_on to ALLOWED_ISSUE_FIELDS whitelist
- Add migration support for existing databases

Project Audit Logging:
- Add user_id and ip_address params to update_project()
- Add user_id and ip_address params to delete_project()
- Log PROJECT_UPDATED event with updated_fields metadata
- Log PROJECT_DELETED event with project name preservation
- Skip audit logging for system operations (no user_id)

Tests:
- 16 TDD tests covering all new functionality
- Verified 72 existing tests still pass (backward compatible)

* refactor(db): address code review feedback for database TODOs

Security:
- Add SQL identifier validation in _add_column_if_not_exists() to prevent
  SQL injection (validates alphanumeric + underscore pattern)

Code Quality:
- Extract _parse_depends_on() helper method for centralized JSON parsing
- Document local imports explaining circular dependency avoidance
- Document audit logging transaction behavior (data consistency over audit)

All 16 tests still passing.

* fix(db): ensure type consistency and audit gating

- _parse_depends_on: Coerce JSON values to strings for consistent API contract
  (json.loads may return ints, but callers expect List[str])
- delete_project: Only emit PROJECT_DELETED audit when rows_affected > 0
  (mirrors update_project behavior, avoids logging no-op deletions)

Author: frankbria

codeframe/persistence/repositories/issue_repository.py

@@ -29,6 +29,7 @@
     "priority",
     "workflow_step",
     "completed_at",
+    "depends_on",
 }
 
 
@@ -154,6 +155,9 @@ def get_issues_with_tasks(self, project_id: int, include_tasks: bool = False) ->
         for issue_row in issue_rows:
             issue_dict = dict(issue_row)
 
+            # Parse depends_on from JSON using centralized helper
+            depends_on = self._parse_depends_on(issue_dict.get("depends_on"))
+
             # Format issue according to API contract
             formatted_issue = {
                 "id": str(issue_dict["id"]),
@@ -162,7 +166,7 @@ def get_issues_with_tasks(self, project_id: int, include_tasks: bool = False) ->
                 "description": issue_dict["description"] or "",
                 "status": issue_dict["status"],
                 "priority": issue_dict["priority"],
-                "depends_on": [],  # TODO: Parse from database if stored
+                "depends_on": depends_on,
                 "proposed_by": "agent",  # Default for now
                 "created_at": self._ensure_rfc3339(issue_dict["created_at"]),
                 "updated_at": self._ensure_rfc3339(issue_dict["created_at"]),  # Use created_at for now
@@ -306,6 +310,33 @@ def get_issue_with_task_counts(self, issue_id: int) -> Optional[IssueWithTaskCou
 
 
 
+    def _parse_depends_on(self, depends_on_str: Optional[str]) -> List[str]:
+        """Parse depends_on JSON string into a list of dependency IDs.
+
+        The depends_on field is stored as a JSON array of issue/task IDs.
+        IDs may be stored as integers or strings in the JSON; this method
+        coerces all values to strings for consistency with API contracts.
+        Handles NULL values, invalid JSON, and non-list JSON gracefully.
+
+        Args:
+            depends_on_str: JSON string from database, or None
+
+        Returns:
+            List of dependency IDs as strings, or empty list if parsing fails
+        """
+        if not depends_on_str:
+            return []
+        try:
+            parsed = json.loads(depends_on_str)
+            # Ensure it's a list - non-list JSON returns empty list
+            if isinstance(parsed, list):
+                # Coerce all values to strings for consistent API contract
+                return [str(x) for x in parsed]
+            return []
+        except (json.JSONDecodeError, TypeError):
+            # Invalid JSON returns empty list
+            return []
+
     def _row_to_issue(self, row: Union[sqlite3.Row, aiosqlite.Row]) -> Issue:
         """Convert a database row to an Issue object.
 

codeframe/persistence/repositories/project_repository.py

@@ -191,15 +191,20 @@ def list_projects(self) -> List[Dict[str, Any]]:
 
 
 
-    def update_project(self, project_id: int, updates: Dict[str, Any]) -> int:
+    def update_project(
+        self,
+        project_id: int,
+        updates: Dict[str, Any],
+        user_id: Optional[int] = None,
+        ip_address: Optional[str] = None,
+    ) -> int:
         """Update project fields.
 
-        TODO(Issue #132): Add audit logging for PROJECT_UPDATED event.
-        Requires adding user_id parameter and updating all callers.
-
         Args:
             project_id: Project ID to update
             updates: Dictionary of fields to update
+            user_id: ID of user performing update (for audit logging)
+            ip_address: Client IP address (for audit logging)
 
         Returns:
             Number of rows affected
@@ -238,24 +243,70 @@ def update_project(self, project_id: int, updates: Dict[str, Any]) -> int:
         cursor.execute(query, values)
         self.conn.commit()
 
-        return cursor.rowcount
+        rows_affected = cursor.rowcount
 
+        # Log project update if user_id is provided
+        # NOTE: Audit logging happens after commit, so if audit fails, the update
+        # is already persisted. This is intentional - we prefer data consistency
+        # over audit completeness. Failed audits are logged but don't roll back.
+        if user_id is not None and rows_affected > 0:
+            # Import locally to avoid circular dependency: audit_logger -> database -> project_repository
+            from codeframe.lib.audit_logger import AuditLogger, AuditEventType
+            audit = AuditLogger(self._database if self._database else self)
+            audit.log_project_event(
+                event_type=AuditEventType.PROJECT_UPDATED,
+                user_id=user_id,
+                project_id=project_id,
+                ip_address=ip_address,
+                metadata={"updated_fields": list(updates.keys())},
+            )
 
+        return rows_affected
 
-    def delete_project(self, project_id: int) -> None:
-        """Delete a project.
 
-        TODO(Issue #132): Add audit logging for PROJECT_DELETED event.
-        Requires adding user_id parameter to distinguish user-initiated
-        deletions from automatic cleanup operations.
+
+    def delete_project(
+        self,
+        project_id: int,
+        user_id: Optional[int] = None,
+        ip_address: Optional[str] = None,
+    ) -> None:
+        """Delete a project.
 
         Args:
             project_id: Project ID to delete
+            user_id: ID of user performing deletion (for audit logging)
+            ip_address: Client IP address (for audit logging)
         """
+        # Get project name before deletion for audit log
+        project_name = None
+        if user_id is not None:
+            project = self.get_project(project_id)
+            if project:
+                project_name = project.get("name")
+
         cursor = self.conn.cursor()
         cursor.execute("DELETE FROM projects WHERE id = ?", (project_id,))
         self.conn.commit()
 
+        rows_affected = cursor.rowcount
+
+        # Log project deletion if user_id is provided and deletion actually occurred
+        # NOTE: Audit logging happens after commit, so if audit fails, the deletion
+        # is already persisted. This is intentional - we prefer data consistency
+        # over audit completeness. Failed audits are logged but don't roll back.
+        if user_id is not None and rows_affected > 0:
+            # Import locally to avoid circular dependency: audit_logger -> database -> project_repository
+            from codeframe.lib.audit_logger import AuditLogger, AuditEventType
+            audit = AuditLogger(self._database if self._database else self)
+            audit.log_project_event(
+                event_type=AuditEventType.PROJECT_DELETED,
+                user_id=user_id,
+                project_id=project_id,
+                ip_address=ip_address,
+                metadata={"name": project_name} if project_name else None,
+            )
+
 
 
     def _row_to_project(self, row: Union[sqlite3.Row, aiosqlite.Row]) -> Project:

codeframe/persistence/schema_manager.py

@@ -63,11 +63,69 @@ def create_schema(self) -> None:
         # Create all indexes
         self._create_indexes(cursor)
 
+        # Apply schema migrations for existing databases
+        self._apply_migrations(cursor)
+
         self.conn.commit()
 
         # Ensure default admin user exists
         self._ensure_default_admin_user()
 
+    def _apply_migrations(self, cursor: sqlite3.Cursor) -> None:
+        """Apply schema migrations for existing databases.
+
+        Handles adding new columns to existing tables.
+        These are idempotent - safe to run multiple times.
+        """
+        # Migration: Add depends_on column to issues table (cf-207)
+        self._add_column_if_not_exists(
+            cursor, "issues", "depends_on", "TEXT"
+        )
+
+    def _add_column_if_not_exists(
+        self,
+        cursor: sqlite3.Cursor,
+        table_name: str,
+        column_name: str,
+        column_type: str,
+        default_value: str = None,
+    ) -> None:
+        """Add a column to a table if it doesn't exist.
+
+        Args:
+            cursor: SQLite cursor
+            table_name: Table to modify
+            column_name: Column to add
+            column_type: SQLite column type
+            default_value: Optional default value for the column
+
+        Raises:
+            ValueError: If table_name, column_name, or column_type contain invalid characters
+        """
+        # SECURITY: Validate identifiers to prevent SQL injection.
+        # Only alphanumeric + underscore allowed (standard SQL identifier rules).
+        import re
+        identifier_pattern = re.compile(r'^[a-zA-Z_][a-zA-Z0-9_]*$')
+        for name, value in [("table_name", table_name), ("column_name", column_name), ("column_type", column_type)]:
+            if not identifier_pattern.match(value):
+                raise ValueError(f"Invalid SQL identifier for {name}: {value}")
+
+        # Check if column exists
+        cursor.execute(f"PRAGMA table_info({table_name})")
+        columns = {row[1] for row in cursor.fetchall()}
+
+        if column_name not in columns:
+            # Add the column
+            if default_value is not None:
+                cursor.execute(
+                    f"ALTER TABLE {table_name} ADD COLUMN {column_name} {column_type} DEFAULT {default_value}"
+                )
+            else:
+                cursor.execute(
+                    f"ALTER TABLE {table_name} ADD COLUMN {column_name} {column_type}"
+                )
+            logger.info(f"Added column {column_name} to {table_name}")
+
     def _create_auth_tables(self, cursor: sqlite3.Cursor) -> None:
         """Create authentication tables (fastapi-users compatible)."""
         cursor.execute(
@@ -198,6 +256,7 @@ def _create_issue_task_tables(self, cursor: sqlite3.Cursor) -> None:
                 status TEXT CHECK(status IN ('pending', 'in_progress', 'completed', 'failed')),
                 priority INTEGER CHECK(priority BETWEEN 0 AND 4),
                 workflow_step INTEGER,
+                depends_on TEXT,
                 created_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP,
                 completed_at TIMESTAMP,
                 UNIQUE(project_id, issue_number)