Merge fix/105-per-request-session -> main #140

Closed
Mark wants to merge 2 commits from fix/105-per-request-session into main
Owner

Auto-opened batch merger.

Auto-opened batch merger.
- app/modules/base.py: declare name/description/requires_config as ClassVar so
  EvidenceModule subclass redeclarations stop tripping '[misc] Cannot override
  instance variable with class variable' (clears 158 [misc] errors).
- Six SQLAlchemy order_by() sites: cast(ColumnElement[Any], <model_column>)
  at the use site to bridge stub/runtime disagreement in
  app/services/investigations/state_machine.py:149,
  app/services/investigations/async_executor.py:98,
  app/services/investigations/orchestrator_enhanced.py:327,
  app/services/investigations/orchestrator_v3_llm.py:767,
  app/routers/investigations.py:861, :889.
- app/modules/registry.py:237: replace builtin 'any' with typing 'Any' in
  list_module_details return type annotation.

Out of scope (Real-bucket / sibling card #116): [assignment], [return-value],
[attr-defined], [call-arg]; PK-column nullability ([arg-type] int | None -> int)
lives in the Real-bug bucket and is owned by #116.

Refs #109, #117
fix(tests): per-request session via QueuePool + WAL (closes #105)
Some checks failed
CI / Backend Lint & Format (pull_request) Failing after 4s
CI / Golden Fixture Validation (pull_request) Has been skipped
CI / Backend Tests (pull_request) Has been skipped
CI / Backend Security Tests (pull_request) Has been skipped
CI / Frontend Lint (pull_request) Failing after 22s
CI / Frontend Tests (pull_request) Has been skipped
CI / Dependency Security Scan (pull_request) Has been skipped
Performance Regression Tests / Performance Regression Detection (pull_request) Failing after 6s
Stress Tests / Stress & Pipeline Tests (pull_request) Failing after 6s
Stress Tests / Full Data Pipeline Validation (pull_request) Has been skipped
CI / Frontend E2E Tests (pull_request) Has been skipped
Performance Regression Tests / Compare with Baseline (pull_request) Has been skipped
CI / All Checks Passed (pull_request) Failing after 1s
0a9077925a
The shared-session override in tests/conftest.py:84-92 yielded a single
SQLAlchemy Session to every concurrent FastAPI request. Sessions are not
thread-safe, so concurrent requests corrupted each other's identity map and
transaction state, producing non-deterministic PendingRollbackError (500)
and spurious 403 'Insufficient case access' responses.

14 of the 14 concurrency tests in tests/integration/test_integration_concurrent.py
were affected (the issue body says '13 of 14 fail on pristine main'; since
#96/#101 landed, 12/14 fail — the fix below addresses all of them).

Fix:
- Engine: switched from StaticPool+in-memory to QueuePool+file-backed SQLite
  with WAL journal mode. Each per-request session now gets its own connection
  from the pool, which is the safe shape under concurrency (SQLAlchemy
  Session and DBAPI connections are not thread-safe to share).
- Per-test isolation: the old 'wrap each test in a transaction that gets
  rolled back at teardown' pattern doesn't compose with per-request
  connections, so it is replaced by an autouse clean_db fixture that
  truncates all tables before each test (FK enforcement is disabled
  during the truncate).
- db_session: rebinds to the engine (no longer to a per-test connection),
  so committed data is visible to per-request sessions through the shared
  SQLite database.
- db_session_factory: yields a fresh Session per FastAPI request, with an
  after_commit hook that removes corresponding ORM objects from
  db_session.identity_map after an API delete. The hook preserves the
  original shared-session behavior expected by tests like
  test_delete_case_*_succeeds (which assert db_session.get(...) is None
  after an API delete).

Test updates (tests/integration/test_integration_concurrent.py): several
concurrency tests had assertions written under the pre-#68 access model
where any analyst could edit any case. They now grant explicit
CaseMembership to collaborators via _grant_case_access, matching the
case-level access control (#68). test_user_cannot_modify_other_users_case
was renamed-stale (name says 'cannot' but asserted 200); assertion flipped
to 403 to match the test's stated intent.

Verification:
- test_integration_concurrent.py: 14/14 pass (10/10 runs, stable)
- test_api_cases.py: 47/47 pass (3/3 runs, stable)
- Full integration suite: 252 pass, 37 fail (vs pristine main 239 pass, 50
  fail — net +13). All 37 remaining failures are pre-existing (verified
  by diff against pristine main).
- New failures introduced by this change: 0.
Mark closed this pull request 2026-09-21 01:06:39 +00:00
Some checks failed
CI / Backend Lint & Format (pull_request) Failing after 4s
CI / Golden Fixture Validation (pull_request) Has been skipped
CI / Backend Tests (pull_request) Has been skipped
CI / Backend Security Tests (pull_request) Has been skipped
CI / Frontend Lint (pull_request) Failing after 22s
CI / Frontend Tests (pull_request) Has been skipped
CI / Dependency Security Scan (pull_request) Has been skipped
Performance Regression Tests / Performance Regression Detection (pull_request) Failing after 6s
Stress Tests / Stress & Pipeline Tests (pull_request) Failing after 6s
Stress Tests / Full Data Pipeline Validation (pull_request) Has been skipped
CI / Frontend E2E Tests (pull_request) Has been skipped
Performance Regression Tests / Compare with Baseline (pull_request) Has been skipped
CI / All Checks Passed (pull_request) Failing after 1s

Pull request closed

Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
Mark/osint-hub!140
No description provided.