fix: isolate DB connection per tool call (backport #56)

[mergify]

LangGraph runs sync tools via asyncio.to_thread, which copies the calling context into the worker thread. Frappe's frappe.local is contextvar-backed, so tool worker threads inherited the same pymysql connection as the agent thread; concurrent tool calls (notably the document-planner subagent) corrupted the shared connection with "Packet sequence number wrong" / InterfaceError(0, '') errors.

Each tool call now opens a private connection, restores the session user, and closes it afterwards. The document-planner subagent tools are now wrapped too (they were previously passed raw).

---

This is an automatic backport of pull request #56 done by Mergify.

[greptile-apps]

Confidence Score: 4/5

Safe to merge; the core isolation logic is sound and well-tested, with two minor edge-case rough edges worth a follow-up.

The frappe.connect() call sits outside the try/finally block, so a partial connect failure could leak a connection and leave frappe.local.db pointing at a dead object. The async wrapper also calls the synchronous frappe.connect() on the event loop thread, briefly stalling other coroutines. Neither affects the correctness of the primary concurrency fix.

ask_alyf/ask_alyf/toolset.py — specifically the placement of frappe.connect() relative to the try block in _isolated_db(), and the async wrapper's use of the sync context manager.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant EL as Event Loop (agent thread)
    participant TH as Worker Thread (asyncio.to_thread)
    participant ISO as _isolated_db()
    participant PDB as Private pymysql conn
    participant IDB as Inherited pymysql conn

    EL->>TH: "asyncio.to_thread(wrapped_sync_tool)<br/>(copies contextvar snapshot)"
    TH->>ISO: enter _isolated_db()
    ISO->>ISO: "inherited_db = frappe.local.db"
    ISO->>PDB: "frappe.connect(set_admin_as_user=False)"
    ISO->>TH: yield - tool runs against PDB
    alt success
        ISO->>PDB: frappe.db.commit()
    else exception
        ISO->>PDB: frappe.db.rollback() suppressed
        ISO->>TH: re-raise
    end
    ISO->>PDB: frappe.local.db.close()
    ISO->>TH: "frappe.local.db = inherited_db"
    TH-->>EL: return / raise
    Note over EL: EL context still has frappe.local.db = IDB (untouched)

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant EL as Event Loop (agent thread)
    participant TH as Worker Thread (asyncio.to_thread)
    participant ISO as _isolated_db()
    participant PDB as Private pymysql conn
    participant IDB as Inherited pymysql conn

    EL->>TH: "asyncio.to_thread(wrapped_sync_tool)<br/>(copies contextvar snapshot)"
    TH->>ISO: enter _isolated_db()
    ISO->>ISO: "inherited_db = frappe.local.db"
    ISO->>PDB: "frappe.connect(set_admin_as_user=False)"
    ISO->>TH: yield - tool runs against PDB
    alt success
        ISO->>PDB: frappe.db.commit()
    else exception
        ISO->>PDB: frappe.db.rollback() suppressed
        ISO->>TH: re-raise
    end
    ISO->>PDB: frappe.local.db.close()
    ISO->>TH: "frappe.local.db = inherited_db"
    TH-->>EL: return / raise
    Note over EL: EL context still has frappe.local.db = IDB (untouched)

Loading

_{Reviews (1): Last reviewed commit: "fix: isolate DB connection per tool call..." | Re-trigger Greptile}

[greptile-apps]

Potential connection leak if frappe.connect() raises

frappe.connect(set_admin_as_user=False) sits between capturing inherited_db and entering the try/finally block. Frappe's connect() assigns to frappe.local.db early in its implementation; if it then raises (e.g. connection refused, auth failure), the newly assigned connection object is never closed and frappe.local.db is left pointing at a dead/partial connection for the remainder of the thread's lifetime. Moving frappe.connect() inside the try block (with a corresponding cleanup in finally) would cover this path.

[greptile-apps]

Sync frappe.connect() blocks the event loop in async path

_isolated_db() is a synchronous context manager; entering it calls frappe.connect() (a blocking DB handshake) directly on the event loop thread. Async tools run in the event loop, so this stalls all other coroutines for the duration of the TCP+auth round-trip on every tool invocation. The fix for the sync case (asyncio.to_thread) does not apply here. Consider using asyncio.to_thread(frappe.connect, ...) or an async-friendly connection setup in async_wrapper if connection latency becomes a bottleneck.