Add aw check requirements async block check (#171642)

This commit is contained in:
Robert Resch
2026-05-21 01:28:32 +02:00
committed by GitHub
parent 323479ca44
commit 3856405c72
6 changed files with 209 additions and 4 deletions
+109 -3
View File
@@ -180,9 +180,10 @@ Verify that the package's source repository is publicly reachable.
- Any other inconclusive result → ⚠️ with a one-line description.
If `repo_public` resolves to ❌ for a package, **also** mark that
package's `release_pipeline` cell/detail as `` (em dash) and explain
`Skipped because the source repository is not publicly accessible.` —
because the release pipeline cannot be inspected without a public repo.
package's `release_pipeline` and `async_blocking` cells/details as ``
(em dash) and explain `Skipped because the source repository is not
publicly accessible.` — neither check can be performed without a public
repo.
### Check kind: `pr_link`
@@ -258,6 +259,111 @@ host from `package.repo_url`, then apply the corresponding checklist.
3. If no CI config can be retrieved: ⚠️ `Release pipeline could not be
inspected; hosting provider is not GitHub or GitLab.`
### Check kind: `async_blocking`
Verify whether the dependency performs blocking I/O inside async code
paths. Home Assistant runs on a single asyncio event loop, so a library
that exposes an `async` surface must not call blocking APIs from inside
its `async def` functions — that stalls the whole loop. A purely sync
library is fine: Home Assistant integrations are expected to wrap such
calls in an executor.
**Two modes — pick by inspecting `package.old_version`:**
- `old_version` is `null` → **new package**: review the *entire current
source tree*. Nothing about this dependency has been vetted before.
- `old_version` is a string → **version bump**: review only the *diff
between `old_version` and `new_version`*. The previous version was
already accepted, so blocking calls that were present in
`old_version` are not regressions; report only what `new_version`
introduces.
#### Step 1 — Decide whether the library exposes an async surface
Use the `github` MCP tool (for `github.com` repos) or `web-fetch`
(other hosts) on `package.repo_url`. Always inspect the tag /
ref matching `new_version` (e.g. `v{new_version}` or `{new_version}`).
- Locate the top-level package directory (usually named after the
import name, often equal or close to `package.name`).
- Check `pyproject.toml` / `setup.py` / `setup.cfg` / `README*` for
async indicators (`Framework :: AsyncIO` trove classifier, `asyncio`
/ `aiohttp` / `httpx` / `anyio` in dependencies, an async usage
example in the README).
- Grep the package source for `async def`. A handful of `async def`
entries in the public modules is enough to treat the library as
having an async surface.
If the library is **sync-only** (no `async def` in its public modules
and no async framework dependency) → ✅
`Sync-only library; Home Assistant integrations must wrap calls in an
executor.` *This verdict is the same in both modes.*
#### Step 2a — Mode: new package (`old_version` is `null`)
Inspect **every `async def` in the public modules** for blocking
patterns. Walk transitively into helpers the async functions call.
#### Step 2b — Mode: version bump (`old_version` is a string)
Fetch the diff between the two tags and review **only changed lines**:
- GitHub: `GET /repos/{owner}/{repo}/compare/{old_tag}...{new_tag}` via
the `github` MCP tool, or
`https://github.com/{owner}/{repo}/compare/{old_tag}...{new_tag}.diff`
via `web-fetch`. Try the common tag formats in order until one
resolves: `v{version}`, `{version}`, `release-{version}`.
- GitLab: `https://gitlab.com/{namespace}/{project}/-/compare/{old_tag}...{new_tag}.diff`.
- Other hosts: use the project's equivalent compare URL via
`web-fetch`.
If neither tag format resolves on the host, fall back to a full review
(Step 2a) and mention in the detail that the diff was unavailable.
When reviewing the diff, only flag blocking patterns that appear in
**added lines** *inside or reachable from* an `async def`. A blocking
call that existed in `old_version` and is unchanged is not a regression
for this bump.
#### Step 3 — Blocking patterns to look for
In both modes, the patterns to flag inside `async def` bodies are:
- Sync HTTP: `requests.`, `urllib.request`, `urllib3.` direct use,
`http.client.`, sync `httpx.Client(` / `httpx.get(` (NOT the
`AsyncClient`), `pycurl`.
- `time.sleep(` (must be `await asyncio.sleep(`).
- Sync sockets: bare `socket.socket` reads/writes, `ssl.wrap_socket`,
blocking `select.select`.
- File I/O: `open(` / `pathlib.Path.read_*` / `.write_*` for
non-trivial sizes (small one-shot reads during import are
acceptable; reads/writes on the request path are not — prefer
`aiofiles` / executor).
- Sync DB drivers used directly: `sqlite3`, `psycopg2`, `pymysql`,
`pymongo` (sync client), `redis.Redis` (sync client).
- `subprocess.run` / `subprocess.call` / `os.system` (must be
`asyncio.create_subprocess_*`).
A call that is clearly dispatched to an executor
(`run_in_executor`, `asyncio.to_thread`, `anyio.to_thread.run_sync`)
does NOT count as blocking.
#### Step 4 — Verdict
- ✅ — no offending blocking pattern in the surface being reviewed
(whole tree for a new package, added lines for a bump). For a bump,
phrase the detail as `No new blocking calls introduced in
{old_version} → {new_version}.`.
- ⚠️ — blocking calls exist only in sync helpers that the async API
does not call, or only on a clearly non-hot path (e.g. one-shot
setup before the event loop is running). Cite at least one
`<file>:<line>` and explain why it is not on the hot path.
- ❌ — a blocking call is reachable from an `async def` that is part
of the public API on the request / polling path (for a bump: the
call was introduced or moved onto the hot path by this version).
Cite the offending `<file>:<line>` as a clickable link on the repo
host so the contributor can jump to it.
## Notes
- Be constructive and helpful. Reference the inspected workflow / CI
+1
View File
@@ -26,6 +26,7 @@ class CheckKind(StrEnum):
CI_UPLOAD = "ci_upload"
RELEASE_PIPELINE = "release_pipeline"
PR_LINK = "pr_link"
ASYNC_BLOCKING = "async_blocking"
@dataclass(slots=True)
+1
View File
@@ -20,6 +20,7 @@ _CHECK_DISPLAY: tuple[tuple[CheckKind, str], ...] = (
(CheckKind.CI_UPLOAD, "CI Upload"),
(CheckKind.RELEASE_PIPELINE, "Release Pipeline"),
(CheckKind.PR_LINK, "PR Link"),
(CheckKind.ASYNC_BLOCKING, "Async Safe"),
)
_ICONS: dict[CheckStatus, str] = {
+19
View File
@@ -10,6 +10,9 @@ What the runner defers to the LLM (NEEDS_AGENT):
- `pr_link`: presence of the right link in the PR description.
- `release_pipeline`: inspection of the publish workflow when the attestation
was missing or did not identify a recognised CI publisher.
- `async_blocking`: inspection of the dependency source for blocking I/O
inside `async def` functions. Always deferred when the source repo is
available — the deterministic stage cannot read the upstream source.
"""
from .diff import parse_diff
@@ -74,6 +77,7 @@ def run_checks(
)
pkg.checks[CheckKind.REPO_PUBLIC] = fail
pkg.checks[CheckKind.PR_LINK] = fail
pkg.checks[CheckKind.ASYNC_BLOCKING] = fail
elif pkg.repo_url:
pkg.checks[CheckKind.REPO_PUBLIC] = CheckResult(
CheckStatus.NEEDS_AGENT,
@@ -83,6 +87,20 @@ def run_checks(
CheckStatus.NEEDS_AGENT,
"Presence of the required link in the PR description must be verified by the agent.",
)
if pkg.old_version is None:
async_reason = (
"New dependency: agent must review the entire source tree "
"at the new version for blocking I/O inside async functions."
)
else:
async_reason = (
f"Version bump {pkg.old_version}{pkg.new_version}: "
"agent must review only the diff for newly introduced "
"blocking I/O inside async functions."
)
pkg.checks[CheckKind.ASYNC_BLOCKING] = CheckResult(
CheckStatus.NEEDS_AGENT, async_reason
)
else:
fail = CheckResult(
CheckStatus.FAIL,
@@ -90,6 +108,7 @@ def run_checks(
)
pkg.checks[CheckKind.REPO_PUBLIC] = fail
pkg.checks[CheckKind.PR_LINK] = fail
pkg.checks[CheckKind.ASYNC_BLOCKING] = fail
result = CheckRunResult(pr_number=pr_number, packages=packages)
result.rendered_comment = render_comment(result)
return result
@@ -26,6 +26,7 @@ def test_render_all_conclusive_collapses_details() -> None:
CheckKind.CI_UPLOAD: _pass("attestation found"),
CheckKind.RELEASE_PIPELINE: _pass("OIDC via attestation"),
CheckKind.PR_LINK: _pass("link found"),
CheckKind.ASYNC_BLOCKING: _pass("no blocking calls in async"),
},
)
result = CheckRunResult(pr_number=1, packages=[pkg])
@@ -49,6 +50,7 @@ def test_render_needs_agent_emits_generic_placeholders() -> None:
CheckKind.CI_UPLOAD: CheckResult(CheckStatus.WARN, "no attestation"),
CheckKind.RELEASE_PIPELINE: CheckResult(CheckStatus.NEEDS_AGENT, ""),
CheckKind.PR_LINK: CheckResult(CheckStatus.NEEDS_AGENT, ""),
CheckKind.ASYNC_BLOCKING: CheckResult(CheckStatus.NEEDS_AGENT, ""),
},
)
rendered = render_comment(CheckRunResult(pr_number=1, packages=[pkg]))
@@ -57,6 +59,8 @@ def test_render_needs_agent_emits_generic_placeholders() -> None:
assert "{{CHECK_CELL:pkg:release_pipeline}}" in rendered
assert "{{CHECK_DETAIL:pkg:release_pipeline}}" in rendered
assert "{{CHECK_CELL:pkg:pr_link}}" in rendered
assert "{{CHECK_CELL:pkg:async_blocking}}" in rendered
assert "{{CHECK_DETAIL:pkg:async_blocking}}" in rendered
assert "<details open>" in rendered
@@ -55,6 +55,7 @@ def test_runner_attestation_recognised(monkeypatch: pytest.MonkeyPatch) -> None:
assert pkg.checks[CheckKind.RELEASE_PIPELINE].status == CheckStatus.PASS
assert pkg.checks[CheckKind.REPO_PUBLIC].status == CheckStatus.NEEDS_AGENT
assert pkg.checks[CheckKind.PR_LINK].status == CheckStatus.NEEDS_AGENT
assert pkg.checks[CheckKind.ASYNC_BLOCKING].status == CheckStatus.NEEDS_AGENT
assert result.needs_agent is True
@@ -154,9 +155,10 @@ def test_runner_marks_missing_version_as_fail(
pkg = result.packages[0]
assert pkg.checks[CheckKind.CI_UPLOAD].status == CheckStatus.FAIL
assert pkg.checks[CheckKind.RELEASE_PIPELINE].status == CheckStatus.FAIL
# No repo URL → repo_public and pr_link short-circuit to FAIL, not NEEDS_AGENT
# No repo URL → repo_public, pr_link and async_blocking short-circuit to FAIL
assert pkg.checks[CheckKind.REPO_PUBLIC].status == CheckStatus.FAIL
assert pkg.checks[CheckKind.PR_LINK].status == CheckStatus.FAIL
assert pkg.checks[CheckKind.ASYNC_BLOCKING].status == CheckStatus.FAIL
assert result.needs_agent is False
@@ -191,9 +193,81 @@ def test_runner_pypi_found_but_no_repo_url_fails_repo_checks(
pkg = result.packages[0]
assert pkg.checks[CheckKind.REPO_PUBLIC].status == CheckStatus.FAIL
assert pkg.checks[CheckKind.PR_LINK].status == CheckStatus.FAIL
assert pkg.checks[CheckKind.ASYNC_BLOCKING].status == CheckStatus.FAIL
assert "does not advertise" in pkg.checks[CheckKind.REPO_PUBLIC].details
def test_runner_async_blocking_new_package_full_review(
monkeypatch: pytest.MonkeyPatch,
) -> None:
"""A newly added package asks the agent for a full-tree async review."""
_patch_pypi(
monkeypatch,
PypiPackageInfo(
project_urls={"Source": "https://github.com/x/y"},
repo_url="https://github.com/x/y",
file_provenance_urls=["whatever"],
found=True,
),
ProvenanceResult(
has_attestation=True,
publisher_kind="GitHub",
recognized_publisher=True,
detail="ok",
),
)
diff = (
"diff --git a/requirements_all.txt b/requirements_all.txt\n"
"--- a/requirements_all.txt\n"
"+++ b/requirements_all.txt\n"
"@@ -0,0 +1 @@\n"
"+pkg==1.0.0\n"
)
result = run_checks(pr_number=1, diff_text=diff)
pkg = result.packages[0]
assert pkg.old_version is None
detail = pkg.checks[CheckKind.ASYNC_BLOCKING].details
assert pkg.checks[CheckKind.ASYNC_BLOCKING].status == CheckStatus.NEEDS_AGENT
assert "New dependency" in detail
assert "entire source tree" in detail
def test_runner_async_blocking_version_bump_diff_only(
monkeypatch: pytest.MonkeyPatch,
) -> None:
"""A version bump asks the agent to review only the diff for new blocking calls."""
_patch_pypi(
monkeypatch,
PypiPackageInfo(
project_urls={"Source": "https://github.com/x/y"},
repo_url="https://github.com/x/y",
file_provenance_urls=["whatever"],
found=True,
),
ProvenanceResult(
has_attestation=True,
publisher_kind="GitHub",
recognized_publisher=True,
detail="ok",
),
)
diff = (
"diff --git a/requirements_all.txt b/requirements_all.txt\n"
"--- a/requirements_all.txt\n"
"+++ b/requirements_all.txt\n"
"@@ -1 +1 @@\n"
"-pkg==1.0.0\n"
"+pkg==1.1.0\n"
)
result = run_checks(pr_number=1, diff_text=diff)
pkg = result.packages[0]
assert pkg.old_version == "1.0.0"
detail = pkg.checks[CheckKind.ASYNC_BLOCKING].details
assert pkg.checks[CheckKind.ASYNC_BLOCKING].status == CheckStatus.NEEDS_AGENT
assert "1.0.0" in detail and "1.1.0" in detail
assert "diff" in detail
def test_runner_serialises_to_json(monkeypatch: pytest.MonkeyPatch) -> None:
"""The artifact contract: `to_dict()` is JSON-serialisable with expected keys."""
_patch_pypi(