fix: use anyio instead of asyncio primitives so trio runs work - #851

Checked this against inspect (inspect_ai @ 448d5878b) as suggested. Their guidance is in AGENTS.md:

Async Concurrency: Use inspect_ai._util._async.tg_collect() instead of asyncio.gather() for running concurrent async tasks. Use inspect_ai.util.collect() only inside sample subtasks (it adds transcript span grouping).

Four things came out of it, one of which was a real defect in this PR.

1. _async_utils.gather was not a drop-in for asyncio.gather — fixed

An anyio task group wraps even a single failure in an ExceptionGroup, and our gather passed that straight through:

asyncio.gather        : ValueError        (is ValueError? True)
control_arena gather  : ExceptionGroup    (is ValueError? False)   <- before
inspect util.collect  : ValueError        (is ValueError? True)
inspect tg_collect    : ValueError        (is ValueError? True)

Both of inspect's equivalents unwrap; ours did not. That matters for this PR specifically: the seven scorers were raising the original exception via asyncio.gather and would have started raising ExceptionGroup, changing the error recorded against a failed sample.

I checked all 23 gather call sites in the repo — none catches around the call, so nothing was actively broken, but the trap was real and test_gather_propagates_exception had encoded the wrong expectation as correct.

gather now unwraps a single failure and raises several as a group, matching inspect_ai.util.collect. Catching ExceptionGroup rather than BaseExceptionGroup keeps cancellation propagating as a group. Tests updated to assert parity with asyncio.gather rather than the old behaviour.

This also fixes the ~14 pre-existing gather users that predate this PR.

2. The lint approach matches inspect's exactly

Inspect selects "TID251" and configures [tool.ruff.lint.flake8-tidy-imports.banned-api] with per-symbol .msg — the same mechanism I used here, just for typing.TypedDict rather than asyncio. No change needed.

3. No as_completed equivalent exists upstream

Inspect has nothing for streaming results in completion order, so the hand-rolled _as_completed for vllm stands.

4. Deliberately not changed, worth knowing about

• inspect_ai.util.collect is public and a drop-in for our gather, so we could delete _async_utils.gather entirely. I did not, because collect wraps each task in a span(), which changes transcripts across ~23 call sites including monitors and scorers — an eval-comparability change that does not belong in an anyio fix. Inspect's own guidance also reserves collect for sample subtasks and points elsewhere for general use. Worth a follow-up if we want to drop the duplication; tg_collect is the closer match but is private.
• inspect_ai._util._async.run_coroutine is far more capable than the run_sync added here — it handles notebooks via nest_asyncio, detects an already-running loop, and works around an anyio bug where every anyio.run call leaks its return value (see _anyio_run_released, which has an excellent comment explaining the _run_vars reference cycle). Our run_sync is a plain anyio.run. That is no worse than the asyncio.run it replaced — both raise inside a running loop — and the leak is immaterial for CLIs that exit, but if we ever want notebook-safe entry points, that is the prior art.

Full suite green (987 passed), pyright clean, and the end-to-end trio proof still holds.

Delegated to inspect. run_sync now calls inspect_ai._util._async.run_coroutine and get_async_backend calls configured_async_backend. _async_utils is the only module that imports the private inspect_ai._util._async, so if it moves, one file changes.

This turned out to be more than deduplication — the version it replaced was a plain anyio.run, which refuses to start inside a running loop:

from inside a running asyncio loop -> done      # previously RuntimeError
from plain sync code               -> done

So run_sync now works in a notebook, releases anyio's per-loop state afterwards (avoiding a leaked return value per call), and refuses clearly when called from inside a trio task instead of failing obscurely.

A gap this uncovered: two of my new tests were dead in CI

While checking the delegation hadn't broken anything I noticed test_run_sync_honours_the_configured_backend[trio] had gone from PASSED to SKIPPED — and then that it had always been skipped in the default invocation.

conftest.py skips anyio trio variants by matching the nodeid substring "[trio". That also catches a "[trio]" parametrise id on a plain sync test. Both of the backend-parametrised tests I added in this PR were affected, and since CI runs:

uv run pytest -n 8 -m "not skip_ci and not long_running"

with no --runtrio, the assertions that run_sync and the vllm completion iterator honour the trio backend never executed in CI at all. They only ran in the --runtrio pass I was doing locally, which is exactly the sort of thing that looks like coverage and isn't.

Renamed the ids to on-asyncio / on-trio so both cases run in the default invocation. CI count goes 987 → 989.

Worth flagging as a general trap: any @pytest.mark.parametrize value containing trio in this repo silently disappears from CI.

Also filled in a section header in test_async_utils.py that had lost its title on main.

Full suite green (989 passed), pyright clean, trio invocation unchanged against its 102-failure baseline.

Tightened it. Answering the "how" in case it's useful beyond this PR.

The discriminator

anyio's pytest plugin parametrises async tests by injecting an anyio_backend param, so that param — not the test id — is what actually determines the backend. Dumping item.callspec.params at collection makes the distinction obvious:

test_gather_preserves_order[trio]                          anyio_backend='trio'   other={}
test_gather_preserves_order[asyncio]                       anyio_backend='asyncio' other={}
test_run_sync_returns_the_coroutine_result                 anyio_backend=<none>   other={}
test_run_sync_honours_the_configured_backend[trio]         anyio_backend=<none>   other={'backend': 'trio'}

The last line is the bug: a genuine anyio variant and an ordinary parametrize value spelled trio are indistinguishable by nodeid, but trivially distinguishable by param.

def anyio_backend_of(item) -> str | None:
    callspec = getattr(item, "callspec", None)
    if callspec is None:
        return None
    backend = callspec.params.get("anyio_backend")
    if isinstance(backend, tuple):
        backend = backend[0]   # anyio accepts ("trio", {options})
    return backend if isinstance(backend, str) else None

--runtrio then selects anyio_backend_of(item) == "trio" and the default run skips it, with the same "sync tests are skipped in the trio pass" behaviour as before.

Checked before switching

I ran a probe over the whole suite comparing the two classifications:

=== items classified differently: 0 ===

So it is behaviour-preserving today — the only items it would have changed were the two I had already worked around.

Reverted the workaround

The ids=["on-asyncio", "on-trio"] rename from the previous commit is gone; natural asyncio/trio ids are safe now, and keeping them is what demonstrates the fix. Both cases run in the default invocation:

test_run_sync_honours_the_configured_backend[asyncio] PASSED
test_run_sync_honours_the_configured_backend[trio]    PASSED
test_works_under_both_backends[asyncio]               PASSED
test_works_under_both_backends[trio]                  PASSED

Added a guard

control_arena/tests/test_backend_selection.py covers the classifier directly, including {"backend": "trio"} → None, which is the case that deleted two tests from CI. The failure mode here is silent loss of coverage rather than a visible error, so it seemed worth six cheap tests.

Worth knowing

Inspect has the identical substring check in tests/conftest.py (if "[trio" not in item.nodeid), so there was no upstream version to adopt — and the same trap exists there. Might be worth an upstream issue.

CI invocation 989 → 995 (the six guard tests). Trio invocation unchanged against its 102-failure baseline. ruff and pyright clean.

Both review comments addressed in 567dba9 (details in the threads). Then a pass over the whole PR, which turned up four things worth recording.

1. The lint ban was narrower than the documented rule

CONTRIBUTING.md bans asyncio.create_subprocess_exec() and loop.run_in_executor(), but neither was in the ruff list, and nor were the primitives most likely to be reached for next: Lock, Event, Queue, to_thread, timeout, get_event_loop/new_event_loop. A ban list that is a strict subset of the written rule teaches people the rule is advisory. All added, each with the replacement named. No new violations anywhere in the repo — there is now no asyncio usage left outside docstrings.

2. The ban messages pointed at things this PR had superseded

asyncio.run said "Use anyio.run(fn, backend=get_async_backend())", which is exactly the awkward call shape run_sync was added to avoid, and asyncio.as_completed said "use an anyio task group with a memory object stream" — advice to hand-roll the helper that now exists. Both now name the helper.

3. Docs did not mention either new helper

CLAUDE.md, AGENTS.md, and CONTRIBUTING.md all list the asyncio → anyio mapping and all three still pointed at raw anyio.run. Updated, with as_completed_with_exceptions added and a note that ruff enforces the table.

4. gather was discarding __cause__

Follow-on from the ExceptionGroup unwrapping fix earlier in this PR: raise eg.exceptions[0] from None suppresses the group as context, but also clears whatever the failure was originally raised from. A scorer failing with raise ValueError(...) from KeyError(...) reached the caller with __cause__ = None. raise ... from exc.__cause__ suppresses the group and keeps the chain. Tested, and shared between gather and the new helper.

Verification

• ruff format, ruff check, pyright (0 errors).
• 1003 passed in the CI invocation (995 → 1003: five for the new iterator's edge cases, four for process_commits_streaming, minus the tuple-shaped tests that were replaced).
• Trio invocation unchanged against its 102-failure baseline; every test added here passes under both backends.
• Merged main locally (which now includes refactor(iac_fast)!: run through the sandbox API instead of the Docker SDK #852, touching iac_fast/scorers.py, a file this PR also touches): no conflicts, ruff check clean on the merged tree, 1052 passed.

Not changed, for the record

• gather_with_exceptions catches BaseException, so a child's cancellation lands in the results list rather than propagating. The enclosing task group re-raises at its next checkpoint so nothing is actually swallowed, and asyncio.gather(return_exceptions=True) captures a child CancelledError too, so this is faithful to what it replaces. Flagging it as the one remaining place in the module where the semantics are subtle.
• examples/apps_monitor_comparison.py lost its tqdm.asyncio bar around the eval-log writes. It was gather(*atqdm(tasks)), which iterates the list synchronously to unpack it, so the bar filled instantly and never tracked completion. The --- Saving Eval Logs --- line above it is the honest version of the same information.