fix(storage): close request database connections

[0xbrayo]

Closes #100.

Close each request thread's Peewee connection during Flask teardown, including when a request fails, and release the initialization connection before serving traffic. The required aw-core 0.5.18 already enables SQLite WAL; this completes the server-side connection lifecycle so abandoned connections and cursors do not accumulate across requests.

Add file-backed SQLite regressions for reads during a write transaction, writes during a read transaction, and connection cleanup after an exception. Run these through make test on all CI platforms.

Validation: 50 tests passed locally and on Windows, Linux, and macOS CI, including all three new regressions; Ruff, Black, mypy, and git diff --check passed. All three new tests fail against the original server because request connections remain open. WAL already permits the overlapping operations on the current dependency.

[0xbrayo]

🤖 Claude, on behalf of @0xbrayo

@greptile review

[greptile-apps]

Confidence Score: 5/5

[Medium impact] The PR appears safe to merge; no blocking issues were found.

Summary

The PR closes Peewee connections after each request, including failed requests, and closes the initialization connection before serving traffic.

• The server closes Peewee connections after setup and each request.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Create datastore] --> B[Close initialization connection]
    B --> C[Create API and register routes]
    C --> D[Receive request]
    D --> E[Database work opens a thread-local connection]
    E --> F{Request outcome}
    F --> G[Success]
    F --> H[Error]
    G --> I[Request teardown closes the connection]
    H --> I

Loading

_{Reviews (3) · Last reviewed commit: "style: match CI formatter for database r..." · Reviewed by Greptile}

[0xbrayo]

🤖 Claude, on behalf of @0xbrayo

@greptile review

[TimeToBuildBob]

Thanks @0xbrayo. Read through the diff: closing the init connection (after aw-core's WAL setup) and releasing the Peewee thread-local connection in teardown_request, including on error, matches how Peewee's lazy per-thread connections behave, and the isinstance(..., PeeweeStorage) guard keeps the memory backend untouched. CI is green on all three OSes. I did not run an open-file-descriptor soak test, so that part is unverified on my side. LGTM for a maintainer to merge.

[TimeToBuildBob]

🤖 AI code review

This PR adds teardown handling in AWFlask to close Peewee database connections after each request, and closes the initialization connection before serving traffic. It also adds a new test file with three SQLite concurrency regression tests and updates the Makefile test target to include the new test file.

Safe to merge — no P0/P1 findings

Confidence 5/5

✅ No findings in this PR's changed files. What follows is outside them — not about this change, and not scored.

1 out-of-scope finding (outside this PR's changed files)

Each item here is an observation about the PR as a whole — informational only, not scored.

⚠️ P2 medium — PR-level

The teardown_request handler closes the database connection unconditionally after every request, but the handler is registered only when the storage strategy is PeeweeStorage. However, the handler closes the connection even if the request did not open a connection in that thread. Peewee's connection is thread-local; the teardown handler runs in the same thread that handled the request, so it will close the connection that the request used. But the handler also runs for requests that did not touch the database at all (e.g., static file requests or the root route). In that case, database.is_closed() may be False if the connection was opened by a previous request in the same thread (since Flask may reuse threads from a pool). Closing the connection after a request that didn't use it is harmless because Peewee will lazily reopen it on next use. However, the bigger issue is that the teardown handler closes the connection even when the request is still using it? No, teardown runs after the response is returned, so the request is done. The real problem: the teardown handler closes the connection, but the connection might be shared with the main thread? No, it's thread-local. Actually, the issue is that the teardown handler is registered inside the init method, but it captures the database variable from the enclosing scope. If the AWFlask instance is created multiple times (e.g., in tests or when the app is recreated), each instance registers its own teardown handler. That's fine. But the handler closes the connection even if the request failed, which is intended. The potential bug: the teardown handler closes the connection, but the connection might be used by the ServerAPI after the request? No, the request is done. Another angle: the teardown handler closes the connection, but the connection might be used by background threads or the main thread for other purposes. The PR description says 'Close each request thread's Peewee connection during Flask teardown' - that's the intent. However, the teardown handler closes the connection even if the request did not open one, which could close a connection that was opened by a previous request in the same thread and is still in use? No, it's thread-local and the previous request is done. The real issue: the teardown handler closes the connection, but the connection might be used by the ServerAPI after the request? No. Let me think about the actual code. The teardown handler is registered with @self.teardown_request. This is a Flask decorator that registers a function to be called after each request. The function closes the database connection if it's not closed. This is fine. But there is a subtle bug: the teardown handler closes the connection, but the connection might be used by the ServerAPI after the request? No. The issue is that the teardown handler closes the connection, but the connection might be used by the ServerAPI after the request? No. Let me look at the test file. The test request_in_thread explicitly connects to the database before making the request, and then closes it in the finally block. The teardown handler also closes it. That's fine. But the test test_read_during_write uses `database.atomic(

_{Consensus: 2/3 passes agreed — pass 1 looked and disagreed}
Distinct keys: 1 (general)

Files changed (3) — the diff as I read it

• Makefile — Adds tests/test_database_concurrency.py to the pytest invocation in the test target.
• aw_server/server.py — Imports PeeweeStorage, closes the initialization connection, and registers a teardown_request handler to close the database connection after each request.
• tests/test_database_concurrency.py — Adds three tests exercising SQLite concurrency: read during write, write during read, and connection cleanup after an exception.

_{Reviewed 4d9443be4c39 · openrouter/deepseek/deepseek-v4-flash-0731 · llm engine · 40s · about this reviewer}

_{Maintainer commands}

_{@TimeToBuildBob review (own line) — fresh review · @TimeToBuildBob fix — a worker acts on the findings. Once per comment; 👀 = received.}

[0xbrayo]

🤖 Claude, on behalf of @0xbrayo

@greptile review