diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e984a5d..e535b47 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -37,10 +37,13 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v4 + # No `cache: "pip"` here: this job installs with uv, never pip, so the + # cache path is never created and setup-python's post step fails the job + # ("Cache folder path is retrieved for pip but doesn't exist on disk") + # even though mypy passed. - uses: actions/setup-python@v5 with: python-version: "3.12" - cache: "pip" - uses: astral-sh/setup-uv@v5 - run: uv sync --locked --extra dev --extra binary --python 3.12 - run: uv run mypy --config-file pyproject.toml features/ shared/ mcp_server/ rag/ hooks/ wiki/ lifecycle/ graph/ core/ @@ -141,10 +144,10 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v4 + # Same reason as typecheck: uv installs, pip never runs. - uses: actions/setup-python@v5 with: python-version: "3.12" - cache: "pip" - uses: astral-sh/setup-uv@v5 - run: uv sync --locked --extra docs --python 3.12 - name: Build docs (strict — fails on broken refs) diff --git a/autohooks/__main__.py b/autohooks/__main__.py index 203134c..77a3de7 100644 --- a/autohooks/__main__.py +++ b/autohooks/__main__.py @@ -284,18 +284,18 @@ def _close_ariel() -> None: out = asyncio.run(run_inject(cfg, mem, graph, rag, text=ns.text, fmt=ns.format, blocks=ns.blocks, with_recap=ns.with_recap)) print(out) return 0 - except BaseException: - # Close on the failure path too. `_close_ariel()` used to sit after each - # `asyncio.run(...)`, so an exception skipped it; aiosqlite's worker - # threads are non-daemon, `threading._shutdown` joins them at exit, and - # the process then prints a traceback and never returns — systemd keeps - # the unit `active`, so Restart=on-failure never fires. Suppressed here - # only so a failing close cannot replace the command's own exception. + finally: + # Close on EVERY path. aiosqlite worker threads are non-daemon, so an + # unclosed connection makes `threading._shutdown` join forever: the + # process prints its output and never returns — systemd keeps the unit + # `active` (Restart=on-failure never fires) and hooks time out with no + # output. This used to be `except: close; raise` + `else: close`, which + # never ran on success: every command branch ends with `return 0` + # inside the `try`, and a `return` from a `try` body skips `else`. + # Suppressed so a failing close cannot replace the command's own + # exception. with contextlib.suppress(Exception): _close_ariel() - raise - else: - _close_ariel() def _render_recall_md(blocks: list[dict[str, Any]]) -> str: diff --git a/tests/test_autohooks_cli.py b/tests/test_autohooks_cli.py index b17acd0..371e98b 100644 --- a/tests/test_autohooks_cli.py +++ b/tests/test_autohooks_cli.py @@ -133,3 +133,38 @@ async def boom(*args, **kwargs): cli.main(["recall", "--config", str(cfg)]) assert closed == ["closed"] + + +def test_main_closes_connections_on_success(tmp_path, monkeypatch): + """A successful command must close ariel's connections before returning. + + Every command branch ends with `return 0` inside the `try`, and a `return` + from a `try` body skips the `else:` clause entirely. The close therefore + never ran on the success path — the exact path every hook takes. The + non-daemon aiosqlite worker then blocked interpreter shutdown, so + `autohooks inject` printed its block and hung until the 120 s watchdog + killed it. Hooks time out at 20 s, so the injection was silently lost. + """ + from autohooks import appctx + from shared.connection import connection_manager + + cfg = tmp_path / "agent.yaml" + cfg.write_text(f"data_dir: {tmp_path}\nuser_id: default\nlayer: user\n", encoding="utf-8") + monkeypatch.setenv("ARIEL_CLI_GUARD", "0") + + closed: list[str] = [] + + async def fake_close_all() -> None: + closed.append("closed") + + monkeypatch.setattr(connection_manager, "close_all", fake_close_all) + monkeypatch.setattr(appctx, "build_app_context", lambda: object()) + monkeypatch.setattr(appctx, "resolve_layer", lambda app, layer, uid: (None, None, None)) + + async def ok(*args, **kwargs): + return [] + + monkeypatch.setattr("features.recall.recall_protocol", ok) + + assert cli.main(["recall", "--config", str(cfg), "--query", "x"]) == 0 + assert closed == ["closed"], "connections must be closed on the success path too"