Skip to content

BUG: UnitOfWork correctness -- missing commit, connection held across await, double-commit #63

Description

@echel0nn

Labels: bug, database, resilience
Severity: S1. Distinct from the team_context isolation gap (#53). These are transaction-lifecycle bugs.

Root cause (confirmed): UnitOfWork wraps async_session_scope(), which does async with factory() as session: yield session. SQLAlchemy AsyncSession.__aexit__ closes the session and rolls back any uncommitted transaction -- there is no commit-on-success. So any UoW block that add()s or mutates but forgets commit() silently discards the write with no error.

Finding (path:line) Class Issue
modules/vr/api_router.py:2749-2756 (refresh_target_source) Missing commit Loads VRTargetRecord, sets mcp_handles_json["audit_mcp_index_id"] to the new id, calls uow.session.add(row) -- but never await uow.commit(). The block exits, the write rolls back. The handler still returns index_id: new_index_id, so the operator sees success while the DB keeps the stale id -> later tool calls fail with "index not found". Confirmed silent data loss.
platform/runtime/orchestrator.py:141-199 (AILAPlatform.handle) Held across await Opens async with async_session_scope() as session: then holds it across router.route(session, query) -> model.chat_json('routing', ...) (1-5s LLM call, routing/router.py:151), module dispatch, and _finalize_run. A pooled DB connection is pinned for the whole request incl. the LLM round-trip (idle during it) -> pool exhaustion under concurrent interactive load
modules/forensics/workflow/states/deep_analysis.py:71-94 Held across await UoW wraps a for-loop calling _analyze_single_file() per target, each running 4-5 SSH commands (sha256sum/strings/floss/capa/ghidra, up to minutes). DB connection pinned for the entire loop. The collection state shows the correct pattern (SSH outside UoW, per-artifact short UoW)
modules/forensics/api_router.py:3726-3735 (cancel_investigation) + platform/tasks/storage.py:80-93 (set_cancelled) Double-commit desync set_cancelled(uow.session, ...) commits TaskRecord.status=CANCELLED internally, then the handler mutates inv.status and calls uow.commit(). If the second commit fails, TaskRecord is CANCELLED but InvestigationRunRecord stays running -> the 3-sources-of-truth desync. A repository method that commits the caller's session breaks caller atomicity everywhere it is used
modules/forensics/api_router.py:1181-1210,1294-1325 (check_readiness) Held across await After commit, task_queue.submit() (Redis I/O) is awaited inside the still-open UoW -> connection held across the Redis round-trip

Calibration (verified clean -- no rewrite needed): malware agents (branch_manager FOR UPDATE locks released in-block, LLM calls outside UoW after fix §89), the workflow engine (per-step sessions), vr vuln_researcher/synthesis_agent. The bug is localized to the sites above.

Acceptance: refresh_target_source commits; handle() and deep_analysis collect outside the session and open a short UoW only for the DB write; set_cancelled does not commit a caller-supplied session (caller owns the transaction); readiness enqueue happens after the UoW closes. Add an honesty-audit rule: a UoW block containing session.add/delete/attribute-mutation with no commit() before exit is a finding.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions