Skip to content

fix(storage): reuse connection per decision cycle - #656

Open
be-student wants to merge 3 commits into
DobermanCore:mainfrom
be-student:fix/sqlite/decision-cycle-connection
Open

fix(storage): reuse connection per decision cycle#656
be-student wants to merge 3 commits into
DobermanCore:mainfrom
be-student:fix/sqlite/decision-cycle-connection

Conversation

@be-student

Copy link
Copy Markdown
Contributor

Pull Request

Slice

What this PR does

Wraps each proxy decision in a task-local SQLite connection scope. Nested storage helpers reuse that connection, while standalone calls, other repositories, and concurrent decisions keep independent lifecycles.

Tests added (run in CI)

  • One complete call_tool() uses one physical connection.
  • Shared-connection setup failure denies before forwarding and sanitizes the error.
  • Nested, cross-repository, and concurrent scope behavior.

Changelog

  • changelog.d/<PR>.fixed.md fragment added in the follow-up commit

Public-release safety (doberman-core only)

  • Contains nothing from the "not allowed" list
  • Core still builds/tests/runs with no enterprise package installed

Security checklist

  • Fails closed on error / uncertainty
  • No secret, full file, or unredacted prompt logged or committed
  • Guardrail and learning behavior is unchanged
  • Every BLOCK/AUTH behavior is unchanged
  • doberman-core does not import doberman_enterprise

Edge cases covered / Deviations from plan / Risks introduced

  • Full local run: 5,528 passed, 5 skipped, 1 xfailed, 92.24% coverage. Eight unrelated macOS/environment failures reproduced outside the changed proxy/storage path; focused tests and all static CI gates pass.
  • Implemented with Codex assistance; I reviewed the diff and validation results.

Closes #597

@fu351

fu351 commented Sep 8, 2026

Copy link
Copy Markdown
Collaborator

Thanks @be-student, the shape here is right: one open per decision instead of twelve, helpers still safe on their own, and a failed setup still fails closed. One thing has to change before I can take it. decide_and_execute now wraps the whole cycle in open_db, and the AUTH challenge inside that cycle runs the prompter on a worker thread through contextvars.copy_context() (challenge.py:410), so the task-local connection travels into that thread. The worker then calls asyncio.run(...) for approval memory and dashboard polling (challenge.py:144, dashboard_prompter.py:103 and 118), and open_db inside those loops hands back the outer loop's aiosqlite connection. An aiosqlite connection belongs to one loop and one thread, and a worker that outlives its join sees ValueError('no active connection') once the outer scope closes it. I reproduced both: the same connection object reused across the thread and a new loop, and the late-worker error. The fix I'd take: keep the reuse for the decision path but make the challenge worker start with _ACTIVE_DB unset (reset it in the worker, or scope open_db to the parts before and after the challenge), plus a test that a challenge worker never sees the outer connection. Your call on which. Ping me when it's pushed.

@fu351 fu351 mentioned this pull request Sep 8, 2026
8 tasks
@fu351

fu351 commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator

Thanks @be-student, holding this one. The shared connection reaches the AUTH path through to_thread and asyncio.run with no timeout on it, so a challenge that takes the long route can leave the chokepoint hung rather than just slow, and that costs more than the handful of opens this saves. If you store the loop that owns the connection and compare asyncio.get_running_loop() before reusing it, falling back to a fresh connection when they differ, I think that closes it. An AUTH-path test with the shared connection would make it obvious either way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

One decided action opens the SQLite DB ~11 times

2 participants