feat(harness): persist server sessions in postgres

2026-07-16 17:31:04 +02:00
parent 8f0154eb9e
commit 5dbb91390f
12 changed files with 1057 additions and 3 deletions
@@ -28,3 +28,124 @@ git diff --check
 The local-vector and preprocess service secret declarations remain for Task 3, which converts
 those services to the same bundle helper. Documentation and smoke command migration is reserved
 for Task 4.
+
+---
+
+# Task 2 report — PostgreSQL session repository
+
+## Scope delivered
+
+- Added `PostgresSessionRepository`, implementing the Task 1 repository contract with a
+  direct PostgreSQL SQLAlchemy connection, transaction-local RLS context, UUIDv4 validation,
+  current artifacts (including `cte_sql:<name>`), append-only decisions, preferences, and
+  content-free deletion tombstones.
+- Added `tht session migrate --database-url URL [--status] --json` and a checksum-protected,
+  advisory-transaction-locked migration runner.
+- Added server session configuration selection. `session_storage.connection` uses direct
+  PostgreSQL TLS modes `verify-ca` or `verify-full`; it does not use PostgREST.
+- Updated packaging and `.gitignore` so session migrations are present in the built wheel.
+- Did not alter Task 3 workflow commands, Pi gate code, or backend code.
+
+## TDD evidence
+
+### RED
+
+Command:
+
+```sh
+cd harness && .venv/bin/pytest tests/test_postgres_session_repository.py tests/test_session_migrate_cmd.py -q
+```
+
+Result before production implementation: `1 failed, 4 errors in 3.89s`.
+
+- Four setup errors were `ModuleNotFoundError: No module named
+  'tht.session.postgres_repository'`.
+- The migration CLI test failed because `tht session migrate` did not exist (`No such command
+  'migrate'`).
+
+### GREEN
+
+Initial focused suite after implementation: `5 passed in 4.18s`.
+
+Final focused verification:
+
+```sh
+cd harness && .venv/bin/pytest \
+  tests/test_session_repository.py \
+  tests/test_postgres_session_repository.py \
+  tests/test_session_migrate_cmd.py \
+  tests/test_vector_migration_packaging.py -q
+```
+
+Result: `12 passed in 5.80s`.
+
+Changed-file lint verification:
+
+```sh
+cd harness && .venv/bin/ruff check \
+  tht/session/postgres_repository.py tht/migrations/sessions tht/config.py \
+  tht/session/repository.py tht/cli/session_cmd.py \
+  tests/test_postgres_session_repository.py tests/test_session_migrate_cmd.py \
+  tests/test_vector_migration_packaging.py
+```
+
+Result: `All checks passed!`.
+
+## Migration and role policy choices
+
+`001_schema.sql` creates only private `thoth_sessions` tables:
+
+- `principals` and `principal_preferences`;
+- `sessions`, with `session_artifacts` and `review_decisions` cascading on session deletion;
+- `audit_log`, which deliberately has no content/detail/metadata column and keeps only action,
+  session UUID, actor identity, owner identity, and timestamp.
+
+`002_security.sql` creates separate `thoth_sessions_runtime` and
+`thoth_sessions_migrator` group roles, explicitly `NOLOGIN NOBYPASSRLS NOSUPERUSER`, revokes
+public access, gives the runtime role only the operations required by the adapter, and enables
+and forces RLS on every table. Owner/admin policies read only transaction-local settings:
+`thoth_sessions.actor_issuer`, `thoth_sessions.actor_subject`, and
+`thoth_sessions.is_admin`. The adapter starts every operation in a transaction, switches to the
+restricted runtime role, sets those settings with `set_config(..., true)`, and uses advisory
+transaction locks for migrations and per-session mutations.
+
+The runtime role remains a `NOLOGIN` group role by design. Deployment must provision a dedicated
+non-superuser LOGIN role and grant it membership, for example:
+
+```sql
+CREATE ROLE thoth_sessions_app LOGIN NOINHERIT PASSWORD '<secret>';
+GRANT thoth_sessions_runtime TO thoth_sessions_app;
+```
+
+This avoids embedding an environment-specific login name or credential in versioned SQL. The
+new integration test proves that this non-superuser membership path can create and read a
+session while the adapter executes as `thoth_sessions_runtime`.
+
+## Security/self-review
+
+- Owner isolation and admin cross-owner reads run against disposable PostgreSQL containers,
+  not Supabase.
+- No table or column includes `embedding`; repository code imports no embedding/vector code;
+  the regression test writes a session artifact under a monkeypatched embedding sentinel.
+- An unauthorized owner receives the same `SessionError` as an absent session, preserving the
+  future backend's 404 mapping boundary.
+- The audit row is inserted before deleting the parent session, so cascades remove all artifact
+  and decision content while the tombstone survives.
+- A security review found and this task fixed the initial `.gitignore` rule that would have
+  excluded `migrations/sessions/*.sql` from Git/wheels. The wheel test now asserts both session
+  migration files and checks both the existing vector CLI and the new session CLI.
+- The review also highlighted runtime login provisioning. It is covered by a non-superuser
+  regression test and documented above; concrete credential/role deployment belongs to Task 7.
+
+## Remaining concerns
+
+- Full `harness/.venv/bin/pytest -q` could not complete in this execution environment: the
+  runner terminated the command after roughly 30 seconds. Captured output reached 44% with no
+  failures before termination; `pgrep` confirmed no pytest process remained. The Task 2 focused
+  suites above completed successfully.
+- `harness/.venv/bin/ruff check .` currently reports 34 pre-existing violations in unrelated
+  test files (for example unused imports in `tests/l0/test_db_connection.py` and semicolon style
+  in `tests/test_phase_effective.py`). The changed-file Ruff command is clean.
+- Task 7 must safely provision the dedicated runtime login/membership and inject its TLS
+  credentials/CA; this task intentionally does not create a deployment-specific LOGIN role or
+  password.