feat(aea-ci): generic check-third-party-hashes + configurable generate-api-docs

[DhairyaPatel7]

Summary

Started as an extension of open-aea-ci-helpers to unblock downstream refactors in open-autonomy#2470; expanded under review to cover three adjacent packaging / testing fixes that turned up during verification. Breakdown below.

1. aea-ci-helpers — new & updated CLI commands

Downstream repos (e.g. open-autonomy) were carrying their own forks of these utilities; the changes here make them reusable.

• New command aea-ci check-third-party-hashes — verifies a local packages/packages.json third_party map against one or more upstream repos, supplied as --upstream owner/repo@version (repeatable, @v prefix tolerated). A package is OK if any upstream has a matching hash; reachable upstream with a wrong hash = mismatch; package absent from every reachable upstream = missing (exit 1); all upstreams unreachable = exit 1; a single flaky upstream is tolerated as long as another responds. Uses aea.helpers.http_requests (no requests dep). Errors/warnings go to stderr via click.echo(err=True).
• aea-ci generate-api-docs now configurable — new options --source-dir, --packages-dir, --plugins-dir, --docs-dir, --default-package, --ignore-plugin, --ignore-prefix, --parallel. Defaults preserve current open-aea behaviour so check-api-docs and generate-api-documentation tox envs are unaffected. Internals refactored from module-level constants into an ApiDocsConfig dataclass; parallel mode now collects submitted futures and .result()s them so worker exceptions actually surface instead of being silently dropped.
• cli.py imports hoisted to module scope where safe; lazy where a command transitively imports aea.* (so aea-ci-helpers stays installable without open-aea).
• Tests added: 42 pass (17 new covering check_third_party_hashes; rest are smoke tests for every CLI command).

Downstream usage

aea-ci check-third-party-hashes --upstream valory-xyz/open-aea@2.2.0
aea-ci generate-api-docs \
    --source-dir autonomy --packages-dir packages --plugins-dir plugins --parallel

2. PyPI descriptions populated from README (root + 10 plugins)

pypi.org/project/open-aea/#description was empty for 2.2.0 because the root pyproject.toml had no readme key, so Poetry emitted no Description body in PKG-INFO. Every plugin setup.py had the same bug from a different angle: long_description was hardcoded to the same one-liner as description, so each plugin page shipped a minimal description instead of the README.

Fix applied uniformly:

• Root pyproject.toml: readme = "README.md" under [tool.poetry].
• Each of 10 plugin setup.py files: add a local _read_long_description() helper reading the plugin's own README.md.

Verified locally by rebuilding every package and checking PKG-INFO / METADATA now contains Description-Content-Type: text/markdown plus the README body. Plugin description-body lengths went from 46–92 bytes (hardcoded one-liner) to 169–1771 bytes (actual README).

3. test_libp2p DHT integration tests — re-enabled against rebuilt ACN

test_dht.py was blanket-skipped on the libp2p v0.8 → v0.33 bump because the deployed valory/open-acn-node:latest image was still a v0.8 build and wire-incompatible with this branch. The ACN has since been rebuilt at a new Docker repo valory/open-acn (Go 1.24, libp2p v0.33.2), now publishing a :latest alias tracking main courtesy of valory-xyz/open-acn#22.

• Point the Python docker fixture and the CI docker pull step at valory/open-acn:latest.
• While verifying locally, found a second wire-compat issue: libp2p v0.33 refuses to dial /dns4/0.0.0.0/... multiaddrs ("no good addresses"). Switch META_ADDRESS from 0.0.0.0 → 127.0.0.1 in acn_image.py and conftest.py so the entry-peer multiaddrs the fixture builds are actually dialable.
• Replace the blanket skip_acn_docker_mismatch with skip_public_acn_not_upgraded, applied only to the Public variants (those still dial the production fetchai/valory ACN nodes, which haven't been redeployed yet). Local variants and TestDHTRobustness now run.
• Regenerated valory/test_libp2p fingerprint (packages.json + docs/package_list.md + connection.yaml) since the fixture edits changed fingerprinted content.
• Updated CLEANUP.md entry to reflect the new state.

Full pytest run can't complete on macOS (Docker Desktop doesn't expose network="host" ports to the host loopback) — Linux CI exercises the end-to-end path.

4. Small CI hygiene

• bugs.python.org/issue8296 added to check-doc-links-hashes -u skip list. The link is cited in a docstring (aea/skills/tasks.py → auto-generated into docs/api/skills/tasks.md), but the tracker has been deprecated in favour of GitHub Issues and 503s intermittently from GHA IP ranges, causing spurious doc-link failures.

Test plan

• pytest plugins/aea-ci-helpers/tests/ — 42 pass (17 new)
• tox -e black-check -e isort-check -e flake8 -e darglint -e check-copyright
• mypy plugins/aea-ci-helpers/aea_ci_helpers (new files clean; 4 pre-existing errors in check_dependencies.py/check_pyproject.py are unrelated to this PR)
• pylint — 10/10 on all modified/new files
• Packaging: rebuilt root sdist/wheel + each plugin wheel, verified PKG-INFO/METADATA now has a populated Description body
• ACN image: pulled valory/open-acn:latest, booted, confirmed libp2p v0.33.2 and expected peer IDs; verified two-container bootstrap with /dns4/127.0.0.1/ multiaddrs
• aea packages lock clean (hashes regenerated)
• aea-ci check-pyproject still works without aea installed (preserves the CI job that installs only the plugin)
• CI validates across the Python 3.10–3.14 / macOS / Windows matrix
• Tag & release so open-autonomy#2470 can pin the new open-aea-ci-helpers

[Copilot]

Pull request overview

This PR extends open-aea-ci-helpers to be reusable by downstream repos (e.g. open-autonomy) by adding a generic third-party hash verification command and making API docs generation configurable (paths/ignores/parallelism), while keeping defaults compatible with the current open-aea layout.

Changes:

• Added aea-ci check-third-party-hashes command + implementation to validate local packages/packages.json:third_party hashes against one or more upstream repos.
• Refactored generate-api-docs to use an ApiDocsConfig dataclass and added CLI options for source/packages/plugins/docs dirs, ignore lists, default packages, and --parallel.
• Updated CLI wiring/tests and added requests as a dependency.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
plugins/aea-ci-helpers/aea_ci_helpers/check_third_party_hashes.py	New implementation for fetching upstream packages.json and comparing third-party hashes.
plugins/aea-ci-helpers/aea_ci_helpers/cli.py	Adds check-third-party-hashes and extends generate-api-docs with configurable options.
plugins/aea-ci-helpers/aea_ci_helpers/generate_api_docs.py	Refactors API docs generation to be configurable and optionally parallel.
plugins/aea-ci-helpers/setup.py	Adds requests dependency needed for upstream fetching.
plugins/aea-ci-helpers/tests/test_check_third_party_hashes.py	New unit tests for parsing/loading/comparison and exit-code behavior.
plugins/aea-ci-helpers/tests/test_aea_ci_cli.py	Ensures new command is registered in the CLI smoke tests.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[OjusWiZard]

I found one correctness issue not yet covered in the existing review comments.

[DavidMinarsch]

Review

Clean, well-scoped, well-tested (29 tests, 17 new, all mocked — no live network). Naturally extends the recent ci-helpers extraction (06fcc40e1 → c76d6baa2) so open-autonomy can stop forking these utilities. Aligns with the cleanup direction.

Issues worth addressing

1. Parallel mode silently swallows errors (real bug). In generate_api_docs.py:

executor.submit(make_pydoc, dotted_path, doc_file)

The returned Future is dropped. If make_pydoc raises in parallel mode, the exception is lost and the run reports success with missing/empty docs — a regression vs. serial mode where errors propagate. Collect the futures and call .result() (or use concurrent.futures.as_completed / wait(..., return_when=FIRST_EXCEPTION)) before executor.shutdown. Otherwise --parallel is unsafe to use in CI.

2. Tension with the recent "restore eager aea imports" decision (c76d6baa2). This PR introduces lazy aea.* imports inside the generate_api_docs CLI command, justified by "ci-helpers must be usable without open-aea (for check-pyproject)". That's defensible — but the rationale only holds if no other eagerly-imported command in cli.py transitively pulls in aea. Worth verifying that nothing imported at module top (notably the new check_third_party_hashes import + the existing eager imports) drags aea back in. If something does, the lazy hop here is cosmetic.

3. Hardcoded v prefix in the upstream URL.

RAW_PACKAGES_URL = "https://raw.githubusercontent.com/{repo}/v{version}/packages/packages.json"

A user passing --upstream valory-xyz/open-aea@v2.2.0 (the natural tag form) becomes vv2.2.0. Either strip a leading v in Upstream.parse, or document the bare-version requirement in --help. Add a parametrized test for the v-prefixed input.

4. requests is now eagerly imported by the CLI. cli.py does from aea_ci_helpers.check_third_party_hashes import run as run_third_party_hash_check at module scope, which pulls requests (+ urllib3, certifi) into every aea-ci invocation. Minor startup cost, easy to fix by lazy-importing inside the command body — matches the pattern used for other commands and the "keep ci-helpers light" rationale in the new module-level NOTE.

5. Error reporting in run(). Uses print(f"ERROR: {e}") — consider click.echo(..., err=True) for consistency with other commands and to route errors to stderr.

Nits

• should_skip still prints "Skipping, it's in ignore patterns" without saying which file — pre-existing, just noting it's now a method.
• create_subdir removal: confirm nothing imports it (it was a public top-level function); quick rg create_subdir across the repo + downstream pin.
• _submit takes Optional[ThreadPoolExecutor] — once Remote registry #1 is fixed it'll need to return the future, so inlining it may be cleaner.
• fetch_upstream_packages combines upstream dev + third_party — correct (a downstream's third-party is the upstream's first-party), but worth a one-line comment so the next reader doesn't second-guess it.

What's good

• ApiDocsConfig dataclass is the right refactor — defaults preserve open-aea behaviour exactly, so tox -e check-api-docs is unaffected.
• "Match in any upstream is OK" semantics in check_hashes is the right call for downstreams bridging multiple upstreams, and test_match_in_any_upstream_is_ok pins it.
• requests declared in setup.py install_requires — consistent with the recent install_requires hygiene work in 4e2d5b246 / 901763ce0.
• Test isolation via tmp_path + mock.patch("…fetch_upstream_packages") — no network in unit tests.

Recommendation

Approve once #1 (parallel-mode error swallowing) is fixed — that's the only real correctness issue. #2–#5 are cleanup; can be addressed in the same PR or follow-ups.

[LOCKhart07]

Re-review

All prior review points addressed:

• Parallel-mode error swallowing fixed (futures.append(...) + fut.result() in generate_api_docs).
• aea.* import kept lazy — check_third_party_hashes is now lazy-imported inside its CLI handler too, so check-pyproject still runs without open-aea installed.
• Upstream.tag_version strips a leading v, so @v2.2.0 and @2.2.0 both work, with tests.
• requests dependency removed; migrated to aea.helpers.http_requests, and install_requires trimmed accordingly.
• click.echo(..., err=True) used throughout run().
• should_skip messages include the offending path.
• Per-upstream tolerance is the right behaviour and is now directly covered by test_one_unreachable_but_other_matches_is_ok and test_missing_from_reachable_upstreams_yields_exit_1.
• Uniform PyPI long-description fix across the root pyproject.toml and all 10 plugin setup.py files; all plugins have a README.md and a MANIFEST.in that includes it, so sdist/wheel installs won't break.

One non-blocking gap

The parallel-mode fix (my prior #1) has no regression test. The serial-vs-parallel failure semantics are the whole point of the fix, and without a test a future refactor can silently re-break --parallel. A small test that patches make_pydoc (or _dispatch) to raise and asserts generate_api_docs(cfg_with_parallel=True) re-raises would pin the behaviour. Worth adding in this PR or a follow-up; not blocking the merge.

[OjusWiZard]

Looks good overall — nice refactor of aea-ci-helpers for downstream reuse, and the new hash-check command plus test coverage are strong. I read the full changed-file set for this PR and I'm approving with a few non-blocking inline comments for follow-up hardening/clarity.

[OjusWiZard]

Approved after a full changed-file review, leaving the three non-blocking notes here for follow-up:

1. generate_api_docs parallel failure semantics
   Nice improvement collecting the submitted futures and calling .result() here. One subtle issue remains, though: make_pydoc() still catches a broad Exception and returns after printing, so a worker failure never actually reaches the Future as an exception. In practice that means parallel mode can still partially fail while exiting successfully, and the comment above slightly overstates the current guarantee. I'd suggest either letting make_pydoc() raise, or returning structured failures that are aggregated here and converted into a non-zero exit.

2. check_third_party_hashes transport exception breadth
   Worth double-checking the exact exception surface of aea.helpers.http_requests.get(). If it can raise transport-related exceptions beyond ConnectionError, those would bypass the per-upstream tolerance logic and crash the command instead of being reported as a tolerated fetch failure. If the helper really only raises ConnectionError, then this is fine as-is.

3. cli.py readability nit on is_clean
   Minor readability nit: the helper name suggests this returns a "dirty" state, but the local variable is named is_clean, and the subsequent if not is_clean: reads a bit backwards. Not a blocker, but renaming the local variable (or adding a tiny clarifying comment) would make this easier to reason about on review.

[DavidMinarsch]

Post-merge review — flagging a few things worth a follow-up.

Strengths

• Test coverage is solid: 17 new unit tests for check_third_party_hashes covering URL building, status/JSON/transport errors, the dev+third_party merge, missing/mismatched/unreachable upstreams, and the any-match-wins semantics.
• Sensible matching policy: any-upstream-match OK, flaky upstream tolerated, all-unreachable fails. Documented inline and exercised by tests.
• Real bug fix in generate_api_docs parallel mode: futures are now collected and .result()'d in a try/finally so worker exceptions surface instead of being silently dropped. Latent issue in the prior impl.
• Lazy aea.* imports preserved in cli.py so aea-ci-helpers stays installable without open-aea (matters for the bootstrap check-pyproject job). The NOTE at module top is helpful.
• META_ADDRESS 0.0.0.0 → 127.0.0.1 is correctly explained as a libp2p v0.33 dial-side requirement. Comments capture the why.
• PyPI long-description fix verified end-to-end (rebuilt sdist/wheel, inspected PKG-INFO).
• ApiDocsConfig refactor is clean; defaults preserve current behaviour so the existing tox envs are unaffected.

Issues worth a follow-up

1. Possible Local/Public skip-selection bug in test_dht.py — worth eyeballing.
The new code only applies skip_public_acn_not_upgraded inside the else branch:

if base_cls is None:
    test_cls = type(name, bases, {})
else:
    test_cls = type(name, (base_cls,), {})
    test_cls = skip_public_acn_not_upgraded(test_cls)

Previously the skip was unconditional. Intent is Local runs / Public skipped, so this only works if base_cls is None really corresponds to the Local variant in every iteration. If that mapping isn't 1:1, Local tests get skipped — or worse, Public tests run against unreachable production nodes and hang in CI. Worth verifying against the test-class factory loop.

2. valory/open-acn:latest is a mutable tag.
Both the Python fixture and the GHA docker pull step track :latest. An unrelated push to open-acn main could silently break this branch's integration suite. Consider pinning to a versioned tag or a content digest. Same risk existed with open-acn-node:latest; this just inherits it.

3. Upstream.parse doesn't validate version after the v-strip.
@v parses successfully (version == 'v' so the not version check passes), but tag_version == '' builds .../v/packages/packages.json. Falls through to a non-200 at runtime, but a ValueError at parse time would be cleaner. Add a test case.

4. Asymmetric dev-key requirement.
fetch_upstream_packages rejects upstream packages.json without a dev section, but load_local_third_party accepts a local file without third_party. Fine in practice (upstreams always have both), but the asymmetry deserves a one-line comment.

5. Implicit string concatenation reads as accidental in a few spots.
e.g. f\"--default-package {spec!r} must be \" \"'component_type:public_id'\" and the "not a valid packages.json" error message. Works, but reviewers will second-guess it — collapse into single f-strings.

6. bugs.python.org/issue8296 skip is a stopgap.
Cleaner fix is updating the aea/skills/tasks.py docstring to reference the GitHub-Issues replacement (or removing the dead link). The blanket skip will hide future genuine breakage of any other bugs.python.org link.

7. Minor style: load_local_third_party uses open(...) while the surrounding new code prefers Path.read_text().

Security

http_requests.get(url, timeout=30) with a 30s timeout, no new deps, no new shell-outs. URL is built from CLI input but this is a CI-only tool invoked with explicit args — low threat model. OK.

Bottom line

Genuinely useful PR — the new command, the PyPI fix, and the parallel-mode exception-surfacing fix are all real improvements. Main thing to verify is that the test_dht.py if base_cls is None branch actually maps to Local-only. The mutable :latest docker tag is the longest-lived risk.