Skip to content

fix: send AAO directory pagination cursor as cursor#811

Merged
bokelley merged 1 commit into
mainfrom
bokelley/fix-cursor-pagination-hasp
May 22, 2026
Merged

fix: send AAO directory pagination cursor as cursor#811
bokelley merged 1 commit into
mainfrom
bokelley/fix-cursor-pagination-hasp

Conversation

@bokelley
Copy link
Copy Markdown
Contributor

Summary

  • add a separate cursor query parameter to AAO directory authorization fetches
  • keep since scoped to timestamp-based incremental filtering
  • update directory page walking to send next_cursor as ?cursor=...
  • add regression coverage for separate since and cursor semantics

Root Cause

The SDK reused since for pagination tokens even though the directory spec defines since and cursor as separate query parameters. Spec-conformant directories can reject page 2+ requests when opaque cursors arrive in since.

Validation

  • PYTHONPATH=src pytest tests/test_adagents.py -q
  • ruff check src/adcp/adagents.py tests/test_adagents.py
  • pre-commit during commit: black, ruff, mypy, bandit, whitespace, EOF, large files, merge/case conflict, private key checks

@bokelley bokelley force-pushed the bokelley/fix-cursor-pagination-hasp branch from 54989c5 to cc2186c Compare May 22, 2026 18:07
@bokelley bokelley changed the title [codex] Fix AAO directory cursor pagination fix: send AAO directory pagination cursor as cursor May 22, 2026
@bokelley bokelley marked this pull request as ready for review May 22, 2026 18:30
Copy link
Copy Markdown

@aao-ipr-bot aao-ipr-bot Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Fixes a real wire-level bug — spec-conformant directories define since (ISO 8601) and cursor (opaque) as distinct query params, and the SDK was recycling next_cursor values into since=.

Things I checked

  • Public-API delta: new optional kwarg cursor: str | None = None on fetch_agent_authorizations_from_directory (src/adcp/adagents.py:2008) — additive, non-breaking, fix: prefix is the right shape.
  • Pagination loop termination preserved in detect_publisher_properties_divergence (src/adcp/adagents.py:2235) — seen_cursors dedupe + MAX_DIRECTORY_PAGES guard untouched; page 1 sends neither param, page N reuses next_cursor via cursor=.
  • Only in-repo caller of the public function is the divergence sweep itself, already migrated.
  • ad-tech-protocol-expert: sound — confirmed against adcontextprotocol/adcp:docs/aao/directory-api.mdx: since typed ISO 8601, cursor typed opaque, no provision for cursors-on-since. The old "opaque cursor or RFC 3339" docstring was an SDK invention.
  • Tests pin both halves: test_since_passes_through_as_query_string asserts cursor is empty when since is sent; test_cursor_passes_through_as_cursor_query_string asserts the reverse; test_divergence_uses_cursor_param_for_second_page (tests/test_adagents.py:4643-4673) regression-pins the two-page flow with page 1 carrying neither and page 2 carrying cursor=page-2. Belt and suspenders, the right shape.

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

  • Spec also defines status (repeated) and limit query params the SDK still doesn't surface — separate gap, not this PR's scope.
  • Adopters who were passing opaque cursors via since= (bug-compatible with old client) silently keep doing so. Worth a release-note line calling that out so external callers migrate to the new kwarg.

Minor nits (non-blocking)

  1. Docstring tightening is text-only. src/adcp/adagents.py:2029 now documents since as RFC 3339 timestamp, but there's no runtime validator. Fine — wire-side schema validation is the right enforcement point, and tightening here would itself be breaking.

Safe to merge.

@bokelley bokelley merged commit da91183 into main May 22, 2026
36 of 37 checks passed
@bokelley bokelley deleted the bokelley/fix-cursor-pagination-hasp branch May 22, 2026 18:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant