feat(adagents): divergence detector + ?include=properties for directory inverse-lookup (#749 Part 3, adcp#4894)

[bokelley]

Pivot from Part 2

PR #769 (now merged to main) shipped fetch_agent_authorizations_from_directory and closed #746 — that's the Part 2 surface this PR originally proposed. Rather than ship a duplicate wrapper, this PR pivots to the Part 3 work layered on top of #769's API.

What this adds

1. ?include=properties parameter on the directory fetch (adcp#4894).

fetch_agent_authorizations_from_directory now accepts include: list[str] | None. Values are emitted as repeated-key query parameters per the AAO directory-api spec (?include=properties&include=...), not comma-joined.

2. property_ids: list[str] | None field on DirectoryPublisherEntry.

None signals the directory did not return per-publisher IDs (count-only mode); a list signals the directory supports ?include=properties and the IDs are usable for set-diff downstream.

3. detect_publisher_properties_divergence — the divergence detector.

Compares the directory's inline resolution against per-publisher federated adagents.json fetches. Always requests include=["properties"] so the full (publisher_domain, property_id) set-diff lights up where supported. Against older directories that return only properties_authorized counts, falls back to count-comparison.

Safety knobs:

• max_concurrency=20 default — semaphore-capped concurrent federated fetches. Caps the burst against publisher origins at managed-network scale (cafemedia ~6,800 publishers).
• sample_size=200 default — caps the sweep. None opts into a full multi-page sweep, intended for small networks only.

4. New public types: PublisherDivergence, DivergenceReport.

Pydantic models (matching #769's style, not the dataclass design from the original Part 2 sketch).

Count-only fallback contract

When the directory does not return property_ids[], missing_in_inline and missing_in_federated are None. Count-equality does not guarantee set-equality — same-count substitutions are undetectable. Callers should treat an empty divergence report as authoritative only when property_ids is present on the underlying entries.

Out of scope / deferred

• Resolution-paths contract enforcement (per adcp#4827 §Resolution-paths). The federated fetch is documented in the docstring as authoritative when paths disagree; runtime preference selection stays a caller concern.
• Caching / persisted divergence ledger.

Closes #749 Part 3. Part 2 (#746) was closed by #769.

[aao-ipr-bot]

LGTM. Approving on the strength of clean additive shape plus comprehensive test coverage — follow-ups below, none blocking.

Things I checked

• Public-API audit. Three new exports (PublisherDivergence, DivergenceReport, detect_publisher_properties_divergence) plus one new optional field (property_ids: list[str] | None) and one new optional kwarg (include). Purely additive. feat(adagents): prefix is correct — no ! warranted, no discriminator churn, no required→optional flips. tests/fixtures/public_api_snapshot.json updated in lockstep with src/adcp/__init__.py — that snapshot test is load-bearing and it caught the right delta.
• SSRF gate, hostile directory. _probe at src/adcp/adagents.py:2079 runs fetch_adagents(entry.publisher_domain, ...) against a directory-supplied value. fetch_adagents re-runs _validate_publisher_domain + _dns_validate_host per src/adcp/adagents.py:1038, so a directory injecting 169.254.169.254 or an internal hostname gets blocked at the publisher-side gate. The detector doesn't widen the existing SSRF surface.
• ?include= query encoding. Each value goes through quote(v, safe='') at src/adcp/adagents.py:1917-1921. include=["properties&admin=1"] produces include=properties%26admin%3D1 — single value, no extra param. The negative assertion in test_include_multiple_values_repeated_keys (assert "include=properties%2Cfuture" not in raw) is the right anti-test.
• Client ownership. own_client = client is None at adagents.py:2052 + the try/finally wrap covers gather-raises and mid-pagination raises. Caller-provided clients aren't closed; framework-provided ones always are.
• Exception stringification at child_fetch_error. str(exc) on Adagents* and httpx.RequestError exposes the publisher's own domain and HTTP error text only. No Authorization header is sent by _resolve_direct (only User-Agent), so nothing credential-shaped round-trips into the buyer-visible report.
• Concurrency invariant. test_max_concurrency_respected builds an event-gated 20-publisher × 4-slot peak observation. Not timing-based — works on the sem semantics directly. Solid.
• Largest-file rule applied to tests/test_adagents.py (+422) and src/adcp/adagents.py (+190). Read both; cited inline above.

Follow-ups (non-blocking — file as issues)

1. Wire-format confirmation against adcp#4894. The repeated-key form (?include=properties&include=...) is the right REST shape, but most ad-tech / OpenAPI adjacent specs default to comma-joined. ad-tech-protocol-expert flagged this as the load-bearing unverified claim. Worth a quick gh issue view 4894 before the next directory operator hits a mismatch.
2. Directory-driven request amplification. security-reviewer Medium: a hostile directory returning 200 rows all with publisher_domain="victim.example" produces a sustained 20-concurrent burst against one host. The docstring claims max_concurrency "caps the burst against publisher origins" — true for benign directories, not for adversarial ones. Dedupe collected by publisher_domain before fan-out at src/adcp/adagents.py:2076, or add a per-host cap.
3. Pagination loop guard. At src/adcp/adagents.py:2070, sample_size=None opts into a full sweep keyed on next_cursor. A misbehaving directory that re-emits the same cursor (or a cursor that re-matches under since semantics) hangs the sweep. Add a cursor-unchanged break or an iteration cap.
4. property_ids typing. Field is list[str]. If upstream defines a canonical PropertyId type (or adds one later), alias list[str] to it via adcp.types per the CLAUDE.md type-layering rule — keeps the public surface forward-compat without renaming.

Minor nits (non-blocking)

1. Falsy property_id filter. src/adcp/adagents.py:2083-2085 drops federated entries where p.get(\"property_id\") is falsy. Empty-string IDs are invalid upstream but possible on the wire — silent drop could mask a real divergence. Either pass through and let validation catch it, or document the filter behavior in the docstring.
2. Redundant None defaults on error path. adagents.py:2098-2099 sets missing_in_inline=None, missing_in_federated=None explicitly — already the field defaults. Keep for clarity or drop for line economy.
3. Naming. missing_in_inline / missing_in_federated parses correctly with the docstring but trips the reader for half a beat. missing_from_* would read marginally cleaner. Not worth churning the public type for it now — note for the next API revision.

One interesting choice worth calling out: the count-only fallback returns missing_in_*=None rather than [], and the docstring is explicit that callers cannot treat an empty report as authoritative in that mode. That's the right shape — fail-honest beats fail-clean — but worth a runtime warning emitted once per detector invocation when property_ids is None across the sample, so adopters notice they're not getting full set-diff precision.

Safe to merge.

[aao-ipr-bot]

LGTM. Follow-ups noted below — one of them is time-sensitive before the next release-please tag.

The shape is right: directory-side ?include=properties (repeated-key wire form, not comma-joined) lights up the full set-diff, the count-only fallback degrades gracefully against older directories, and the dedupe-by-publisher_domain added in commit 2 closes the amplification path against publisher origins. SSRF gate inheritance is real — fetch_adagents independently re-validates the publisher domain through _check_safe_host (adagents.py:311) and _dns_validate_host (adagents.py:1041), so a hostile directory cannot drive the fan-out at 127.0.0.1, 169.254.169.254, or RFC1918 space via publisher_domain.

Things I checked

• Repeated-key wire form (adagents.py:1924-1932): ?include=properties&include=foo, not comma-joined. Spec match verified against docs/aao/directory-api.mdx (style: form, explode: true). Negative assertion in test_include_multiple_values_repeated_keys catches the comma-joined regression.
• Pagination guards (adagents.py:2087-2095): seen_cursors catches A→B→A cycles, MAX_DIRECTORY_PAGES=1000 catches strictly-monotonic-cursor-forever. Both are load-bearing — test_divergence_aborts_on_repeated_cursor covers the first.
• Dedupe placement (adagents.py:2097-2110): sits between cursor-walk and asyncio.gather. test_divergence_dedupes_collected_by_publisher_domain exercises the 5-rows-1-victim path → 1 fetch.
• SSRF gate on fan-out targets: fetch_adagents re-validates publisher_domain independently. Directory cannot route fetches to internal IPs.
• _probe exception coverage (adagents.py:2142-2149): AdagentsNotFoundError, AdagentsValidationError, AdagentsTimeoutError, httpx.HTTPError, OSError, ValueError — comprehensive against everything fetch_adagents and get_properties_by_agent raise on the documented contract.
• Set-diff determinism (adagents.py:2162-2163): sorted(...) on both sides, test assertions can rely on order.
• Public API surface: three new exports (PublisherDivergence, DivergenceReport, detect_publisher_properties_divergence) added to __init__.py AND tests/fixtures/public_api_snapshot.json:124,257,356. Purely additive — feat(adagents): is the right conventional-commit prefix; no ! needed. property_ids: list[str] | None default-None on DirectoryPublisherEntry keeps existing deserialization paths working.
• max_concurrency semaphore is real: test_max_concurrency_respected uses an event-gated barrier and asserts peak <= 4 against 20 concurrent probes. Load-bearing.
• include= injection surface (adagents.py:1928-1931): values percent-encoded via quote(v, safe=''). Clean.
• ctx_metadata footgun (per CLAUDE.md): not touched. Network helper, not a dispatch path.

Follow-ups (non-blocking — file as issues)

1. Rename missing_in_inline → missing_in_directory before the next release-please tag. ad-tech-protocol-expert: "sound-with-caveats — vocabulary collision." In adcp#4827 §Resolution-paths, inline specifically denotes parent-file inline resolution of publisher_properties selectors (a publisher-side concept). The SDK uses inline here to mean the directory's pre-resolved answer — a different layer. Spec text consistently says directory path vs federated path. Once PublisherDivergence ships in a tagged release the rename becomes breaking. Field is in the public API surface (__init__.py re-export + snapshot) and the next push to main will produce a release-please PR — rename pre-tag.

2. Move dedupe inside the cursor loop so duplicates don't burn sample-size budget. security-reviewer (High, but in the threat-model sense the function never promised coverage guarantees — it's observability tooling, not a security gate, so I'm calling this a follow-up rather than a block): truncation to sample_size=200 runs before dedupe at adagents.py:2081-2110. A directory that returns 200 duplicates of victim.example.com on page 1 collapses to a single probe — amplification still closed, but the divergence detector's sample is steerable. Cleaner: dedupe per-page and break the cursor walk on len(deduped) >= sample_size.

3. Document the count-only-mode caveat in the detect_publisher_properties_divergence docstring, not only PublisherDivergence. ad-tech-protocol-expert confirmed the count-equality-implies-no-divergence behavior is the lossy semantic the spec calls out — the one-shot logger.warning at adagents.py:2117-2123 is good operator-facing instrumentation, but the function-level docstring at adagents.py:2057-2061 should also surface "empty list ≠ no divergence in count-only mode" so adopters don't have to read two docstrings to get the warning.

4. Bound collected memory on the sample_size=None path. security-reviewer (Medium): with sample_size=None and a hostile-but-paging directory, up to 1000 pages × 5 MiB ≈ 5 GiB of DirectoryPublisherEntry rows can land in collected before either guard trips. Either add a MAX_COLLECTED_ENTRIES cap or fold this into the per-page dedupe of follow-up #2.

5. Classify child_fetch_error: str(exc) into a fixed enum. security-reviewer (Low): httpx.RequestError / OSError string forms can include the resolved target IP or local socket path. Acceptable for internal ops use, but if DivergenceReport ever round-trips into a tenant-facing response this leaks egress posture. timeout / not_found / validation / network is enough granularity.

Minor nits (non-blocking)

1. MAX_DIRECTORY_PAGES check ordering (adagents.py:2092). The cap fires on iteration 1001 only after the 1001st fetch_agent_authorizations_from_directory round-trip has completed. Moving the check above the next-iteration body would save one wire call against a misbehaving directory. Not load-bearing — the cap exists for safety, not perf.

2. DivergenceReport should be TypeAlias-annotated (adagents.py:2015). DivergenceReport = list[PublisherDivergence] is a runtime alias; adding from typing import TypeAlias and the annotation helps mypy/IDE tooling resolve it without inferring.

3. releaser() magic number 50 (tests/test_adagents.py:769-772). The for _ in range(50): await asyncio.sleep(0) value is tied to internal event-loop scheduling — a one-liner naming the worst-case task count this is meant to drain (1 releaser + 20 probes + sem internals) would make the test self-documenting.

Approving on the strength of the dedupe close, the SSRF re-validation chain, and the test coverage for both pagination guards.