Merge pull request 'fix: complete user-delete FK lockstep across PG and SQLite schemas' (#1) from fix/user-delete-fk-schema-lockstep into main
Reviewed-on: #1
This commit was merged in pull request #1.
This commit is contained in:
+15
-2
@@ -76,6 +76,11 @@ def _init_sqlite(cfg: CtxConfig) -> sqlite3.Connection:
|
||||
with open(SCHEMA_SQLITE_PATH) as f:
|
||||
conn.executescript(f.read())
|
||||
else:
|
||||
# NOTE: SQLite cannot ALTER a FK constraint in place. Existing dev
|
||||
# databases will NOT pick up FK changes in schema_sqlite.sql (e.g.
|
||||
# ON DELETE SET NULL on user_id columns) — delete $CTXD_HOME/ctxd.db
|
||||
# to recreate from the current schema. Only additive ADD COLUMN
|
||||
# migrations are applied here.
|
||||
# Migration: add metadata_tags column if it doesn't exist
|
||||
try:
|
||||
conn.execute("ALTER TABLE projects ADD COLUMN metadata_tags TEXT DEFAULT '[]'")
|
||||
@@ -205,14 +210,22 @@ def user_delete(conn, user_id: str) -> dict:
|
||||
try:
|
||||
conn.execute(f"DELETE FROM users WHERE user_id = {ph}", (user_id,))
|
||||
return {"ok": True}
|
||||
except (sqlite3.IntegrityError, Exception) as e:
|
||||
except Exception as e:
|
||||
if _is_pg(conn):
|
||||
import psycopg
|
||||
# Roll back unconditionally so a failed DELETE never leaves the
|
||||
# shared connection in an aborted-transaction state (see the 500
|
||||
# cascade fix); only an FK violation maps to the soft response.
|
||||
conn.rollback()
|
||||
if isinstance(e, psycopg.errors.ForeignKeyViolation):
|
||||
return {"ok": False, "error": "user_has_references", "hint": "Inactivate the user instead of deleting."}
|
||||
raise
|
||||
return {"ok": False, "error": "user_has_references", "hint": "Inactivate the user instead of deleting."}
|
||||
# SQLite: only an FK violation is the expected "still referenced" case.
|
||||
# Anything else (I/O error, programming bug, corruption) must propagate
|
||||
# to the global handler as a 500 rather than be masked as a soft 409.
|
||||
if isinstance(e, sqlite3.IntegrityError):
|
||||
return {"ok": False, "error": "user_has_references", "hint": "Inactivate the user instead of deleting."}
|
||||
raise
|
||||
|
||||
|
||||
def user_set_password(conn, user_id: str, password: str):
|
||||
|
||||
@@ -66,7 +66,8 @@ def main():
|
||||
print(f" SKIP {constraint}: already ON DELETE SET NULL")
|
||||
continue
|
||||
|
||||
col = constraint.replace(f"{table}_", "").replace("_fkey", "")
|
||||
# `col` comes straight from the FKS_TO_FIX tuple — don't re-derive it
|
||||
# from the constraint name, which breaks for manually-named constraints.
|
||||
print(f" ALTER {table}.{col} ({constraint}): {row[0]} -> SET NULL")
|
||||
conn.execute(f'ALTER TABLE {table} DROP CONSTRAINT {constraint}')
|
||||
conn.execute(
|
||||
|
||||
@@ -80,7 +80,7 @@ CREATE TABLE context_files (
|
||||
file_path TEXT NOT NULL,
|
||||
content TEXT NOT NULL DEFAULT '',
|
||||
version INTEGER NOT NULL DEFAULT 1,
|
||||
updated_by TEXT REFERENCES users(user_id),
|
||||
updated_by TEXT REFERENCES users(user_id) ON DELETE SET NULL,
|
||||
updated_at TEXT NOT NULL DEFAULT to_char(now() at time zone 'utc', 'YYYY-MM-DD"T"HH24:MI:SS"Z"'),
|
||||
UNIQUE(project_id, file_path)
|
||||
);
|
||||
|
||||
@@ -68,7 +68,7 @@ CREATE TABLE project_context (
|
||||
project_id TEXT PRIMARY KEY REFERENCES projects(project_id) ON DELETE CASCADE,
|
||||
content TEXT NOT NULL DEFAULT '',
|
||||
version INTEGER NOT NULL DEFAULT 0,
|
||||
updated_by TEXT REFERENCES users(user_id),
|
||||
updated_by TEXT REFERENCES users(user_id) ON DELETE SET NULL,
|
||||
updated_at TEXT NOT NULL DEFAULT (strftime('%Y-%m-%dT%H:%M:%SZ', 'now'))
|
||||
);
|
||||
|
||||
@@ -81,7 +81,7 @@ CREATE TABLE context_files (
|
||||
file_path TEXT NOT NULL,
|
||||
content TEXT NOT NULL DEFAULT '',
|
||||
version INTEGER NOT NULL DEFAULT 1,
|
||||
updated_by TEXT REFERENCES users(user_id),
|
||||
updated_by TEXT REFERENCES users(user_id) ON DELETE SET NULL,
|
||||
updated_at TEXT NOT NULL DEFAULT (strftime('%Y-%m-%dT%H:%M:%SZ', 'now')),
|
||||
UNIQUE(project_id, file_path)
|
||||
);
|
||||
@@ -122,7 +122,7 @@ CREATE TABLE change_requests (
|
||||
request_id TEXT PRIMARY KEY,
|
||||
workspace_id TEXT NOT NULL REFERENCES user_workspaces(workspace_id) ON DELETE CASCADE,
|
||||
project_id TEXT NOT NULL REFERENCES projects(project_id),
|
||||
submitted_by TEXT NOT NULL REFERENCES users(user_id),
|
||||
submitted_by TEXT REFERENCES users(user_id) ON DELETE SET NULL,
|
||||
status TEXT NOT NULL DEFAULT 'pending'
|
||||
CHECK (status IN ('pending', 'approved', 'rejected', 'merged')),
|
||||
diff_summary TEXT,
|
||||
@@ -138,7 +138,7 @@ CREATE TABLE change_requests (
|
||||
CREATE TABLE reviews (
|
||||
review_id INTEGER PRIMARY KEY AUTOINCREMENT,
|
||||
request_id TEXT NOT NULL REFERENCES change_requests(request_id) ON DELETE CASCADE,
|
||||
reviewer_id TEXT NOT NULL REFERENCES users(user_id),
|
||||
reviewer_id TEXT REFERENCES users(user_id) ON DELETE SET NULL,
|
||||
decision TEXT NOT NULL CHECK (decision IN ('approved', 'rejected')),
|
||||
comments TEXT,
|
||||
created_at TEXT NOT NULL DEFAULT (strftime('%Y-%m-%dT%H:%M:%SZ', 'now')),
|
||||
@@ -168,7 +168,7 @@ CREATE INDEX idx_snapshots_cleanup ON snapshots (project_id, user_id, created_at
|
||||
-- ============================================================================
|
||||
CREATE TABLE audit_log (
|
||||
entry_id INTEGER PRIMARY KEY AUTOINCREMENT,
|
||||
user_id TEXT NOT NULL REFERENCES users(user_id),
|
||||
user_id TEXT REFERENCES users(user_id) ON DELETE SET NULL,
|
||||
agent_id TEXT NOT NULL DEFAULT 'cli',
|
||||
session_id TEXT,
|
||||
project_id TEXT REFERENCES projects(project_id),
|
||||
@@ -191,18 +191,11 @@ CREATE INDEX idx_audit_project ON audit_log (project_id, created_at);
|
||||
CREATE INDEX idx_audit_agent ON audit_log (agent_id, created_at);
|
||||
CREATE INDEX idx_audit_op ON audit_log (operation, created_at);
|
||||
|
||||
-- Trigger: audit_log is append-only
|
||||
CREATE TRIGGER tr_audit_log_no_update
|
||||
BEFORE UPDATE ON audit_log
|
||||
BEGIN
|
||||
SELECT RAISE(ABORT, 'audit_log is append-only — no updates allowed');
|
||||
END;
|
||||
|
||||
CREATE TRIGGER tr_audit_log_no_delete
|
||||
BEFORE DELETE ON audit_log
|
||||
BEGIN
|
||||
SELECT RAISE(ABORT, 'audit_log is append-only — no deletes allowed');
|
||||
END;
|
||||
-- Note: audit_log append-only enforcement is handled at the application layer.
|
||||
-- DB-level BEFORE UPDATE/DELETE triggers conflict with FK ON DELETE SET NULL
|
||||
-- (deleting a user must set audit_log.user_id to NULL, which is an UPDATE).
|
||||
-- This mirrors schema.sql, which dropped the equivalent PG triggers for the
|
||||
-- same reason. db.py only ever INSERTs into audit_log.
|
||||
|
||||
-- ============================================================================
|
||||
-- FULL-TEXT SEARCH (FTS5)
|
||||
|
||||
@@ -821,8 +821,11 @@ class HTTPServer:
|
||||
except Exception:
|
||||
logger.exception("Rollback failed after request error")
|
||||
if _db.is_integrity_error(e):
|
||||
# Don't leak raw psycopg text (index names, column expressions)
|
||||
# to API callers; log it for operators instead.
|
||||
logger.warning("Integrity error on %s %s: %s", method, path, e)
|
||||
return (409, {"Content-Type": "application/json"},
|
||||
json.dumps({"error": "conflict", "detail": str(e)}))
|
||||
json.dumps({"error": "conflict"}))
|
||||
logger.exception("HTTP error")
|
||||
return (500, {"Content-Type": "text/plain"}, f"Internal error: {e}")
|
||||
|
||||
|
||||
Reference in New Issue
Block a user