Skip to content

fix(claude-admin): after_id cursor pagination + rate-limit backoff fallback - #1955

Merged
cyberdima merged 5 commits into
mainfrom
fix/1953-claude-admin-pagination-and-rate-limit
Jul 28, 2026
Merged

fix(claude-admin): after_id cursor pagination + rate-limit backoff fallback#1955
cyberdima merged 5 commits into
mainfrom
fix/1953-claude-admin-pagination-and-rate-limit

Conversation

@cyberdima

@cyberdima cyberdima commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Summary

  • claude_admin_api_keys, claude_admin_workspaces, claude_admin_workspace_members (parent substream), and claude_admin_invites used an offset/limit (OffsetIncrement) paginator against Anthropic Admin API endpoints that only support after_id/has_more cursor pagination. Since offset is ignored server-side, pages never shrink below page_size and the paginator never terminates. Confirmed on a live sync stuck 65h+ re-reading the same api_keys page. Switched all four to the after_id_paginator already used correctly by claude_admin_users. Fixes claude-admin connector: offset-based pagination never terminates on Anthropic Admin API list endpoints #1953.
  • Also bumps descriptor.yaml to strict semver (ADR-0015) — the prior date-string value (2026.07.25) has a leading zero and is silently rejected by reconcile's classify_bump validation, so version bumps to this connector were never actually being republished to Airbyte.
  • Second commit: Anthropic's 429 responses on these endpoints carry no Retry-After header, so WaitTimeFromHeader always resolves to no wait time and the CDK falls back to a short bounded internal backoff. Once exhausted mid-pagination, the read aborts with no resumable checkpoint and a full-refresh stream restarts from page 1 — for api_keys (~80 pages), any single 429 during the walk causes a full re-read, which repeats indefinitely under sustained rate pressure. Found this while verifying the pagination fix on staging. Added ExponentialBackoffStrategy as a fallback and raised max_retries.

Test plan

  • ManifestDeclarativeSource instantiates cleanly against the pinned CDK 6.60.16 harness
  • Existing claude-admin connector test suite passes (13 passed)
  • Verified live on a staging deployment: hung sync (job stuck 65h+ on api_keys) cancelled, manifest republished, fresh sync completed all 8 streams without the infinite loop recurring

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes

    • Improved handling of Anthropic Admin API rate limits by increasing the retry budget and using an exponential backoff strategy for 429 responses.
    • Corrected timestamps for invitation records by populating created_at from invited_at when needed.
  • Performance

    • Improved synchronization of Admin API resources by switching from offset-based pagination to cursor-based after_id pagination for API keys, workspaces, workspace members, and invites.
    • Increased request limits to retrieve larger batches more efficiently.
  • Chores

    • Updated the connector descriptor version.

@cyberdima
cyberdima requested a review from a team as a code owner July 27, 2026 19:20
@github-actions

Copy link
Copy Markdown
Contributor

⚠️ Regenerate the connectors-ddl snapshot

This PR changes src/ingestion/**. If your change affects any
bronze / silver / gold schema, regenerate the committed DDL snapshot
and include it in this PR.

Prerequisites (details: src/ingestion/scripts/bootstrap-db/README.md):

  • docker + a fresh throwaway ClickHouse 25.7.5 (README "Local ClickHouse for testing")
  • .env from .env.bootstrap.example pointing at it; use the host LAN IP,
    reachable from both the host and connector containers
    (host.docker.internal does not resolve on the macOS host itself)
  • python3.12 or python3.11 on PATH (pinned dbt venv)
  • HubSpot + Salesforce credentials in .env — their discover calls the
    live APIs; without them, apply scripts/connectors-ddl/{hubspot,salesforce}.sql
    to seed their bronze, then run the dbt step
cd src/ingestion/scripts/bootstrap-db
set -a; source pins.env; source .env; set +a
./bootstrap-db.sh connectors-config.yaml   # fresh ClickHouse 25.7.5
./dump-ddl.sh                              # writes scripts/connectors-ddl/*.sql

Commit the resulting scripts/connectors-ddl/*.sql diff. If nothing
changed, no snapshot update is needed. (Regeneration is manual for now.)

@coderabbitai

coderabbitai Bot commented Jul 27, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The Claude Admin connector increases 429 retry capacity, applies exponential fallback backoff, migrates four streams from offset to after_id pagination, maps invite timestamps, removes the offset paginator, and updates descriptor metadata.

Changes

Claude Admin connector updates

Layer / File(s) Summary
Retry handling configuration
src/ingestion/connectors/ai/claude-admin/connector.yaml
The shared 429 handler retries up to 10 times and uses exponential fallback backoff with factor 5.
Cursor pagination migration
src/ingestion/connectors/ai/claude-admin/connector.yaml
API keys, workspaces, workspace members, and invites switch from offset pagination to after_id_paginator with limits of 1000; the offset paginator definition is removed. Invite records populate created_at from invited_at when needed.
Connector version metadata
src/ingestion/connectors/ai/claude-admin/descriptor.yaml
The descriptor version changes from 2026.07.25 to 2026.7.28.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related issues

Possibly related PRs

Suggested reviewers: cyberantonz, mitasovr

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly highlights the main changes: after_id pagination and rate-limit backoff fallback.
Linked Issues check ✅ Passed The four affected streams were switched from offset to after_id cursor pagination as required by issue #1953.
Out of Scope Changes check ✅ Passed The changes stay within the stated objectives, including the descriptor version bump and rate-limit fallback.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/1953-claude-admin-pagination-and-rate-limit

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/ingestion/connectors/ai/claude-admin/connector.yaml`:
- Line 839: Update the claude_admin_invites stream configuration to map the
Admin API’s invited_at field into the schema’s created_at field, while
preserving the existing tenant_id_injection transformation.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 4050be55-34ac-4383-aef3-8b8a14327468

📥 Commits

Reviewing files that changed from the base of the PR and between 65e7b44 and 6cbf76a9ff788964a7770e31ca04ba207793bea2.

📒 Files selected for processing (2)
  • src/ingestion/connectors/ai/claude-admin/connector.yaml
  • src/ingestion/connectors/ai/claude-admin/descriptor.yaml

Comment thread src/ingestion/connectors/ai/claude-admin/connector.yaml
@cyberdima
cyberdima force-pushed the fix/1953-claude-admin-pagination-and-rate-limit branch from 6cbf76a to bbb4755 Compare July 27, 2026 19:27
…sor pagination

Anthropic Admin API list endpoints paginate via after_id/has_more cursors
only; they do not support offset/limit. The claude_admin_api_keys,
claude_admin_workspaces, claude_admin_workspace_members (parent substream),
and claude_admin_invites streams used an OffsetIncrement paginator, which
never terminates against this API: unrecognized offset params are ignored
server-side, so pages never shrink below page_size and the connector loops
indefinitely. Confirmed on a live sync stuck 65h+ re-reading the same
api_keys page. Switched all four to the after_id_paginator already used
correctly by claude_admin_users. Also bumps descriptor.yaml (the prior
value has a leading zero and is silently rejected by reconcile's
classify_bump strict-semver-ish validation, meaning version bumps to this
connector were never actually being republished).

Fixes #1953

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Signed-off-by: Dmitry Saukh <38005371+cyberdima@users.noreply.github.com>
…m restart loop

Anthropic's 429 responses on these endpoints carry no Retry-After header,
so the RATE_LIMITED error handler's WaitTimeFromHeader strategy always
resolves to no wait time and the CDK falls back to its own short, bounded
internal backoff (~5 tries, well under a minute). Once that's exhausted
mid-pagination the read aborts with no resumable checkpoint, so a
full-refresh stream restarts from page 1. For a large multi-page stream
(api_keys, ~80 pages at limit=1000) any single 429 during the walk turns
into a full re-read, and under sustained rate pressure this repeats
indefinitely — observed directly during verification of the #1953 fix.

Adds ExponentialBackoffStrategy as a fallback backoff strategy (used
whenever WaitTimeFromHeader returns None) and raises max_retries so the
connector actually waits out a real rate-limit window instead of giving
up almost immediately.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Signed-off-by: Dmitry Saukh <38005371+cyberdima@users.noreply.github.com>
@cyberdima
cyberdima force-pushed the fix/1953-claude-admin-pagination-and-rate-limit branch from bbb4755 to 38afb36 Compare July 27, 2026 19:30
Admin API returns invited_at for invite timestamps, but the schema
exposes created_at (project-wide convention) with no transformation
backing it, so consumers got null/absent created_at. Addresses
CodeRabbit review comment on PR #1955.

Signed-off-by: Dmitry Saukh <38005371+cyberdima@users.noreply.github.com>
@cyberdima
cyberdima enabled auto-merge July 27, 2026 19:45

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
src/ingestion/connectors/ai/claude-admin/connector.yaml (1)

23-48: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Add a regression test for 429s without Retry-After.

The visible reliability helper always supplies Retry-After: 0, so it only exercises the header-based path. Add a 429-without-header case that recovers successfully; this verifies the new fallback and the target CDK’s strategy chaining.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/ingestion/connectors/ai/claude-admin/connector.yaml` around lines 23 -
48, Add a regression test using the visible reliability helper or equivalent
fixture to return HTTP 429 without a Retry-After header, then verify the
connector recovers successfully through the ExponentialBackoffStrategy fallback
configured alongside WaitTimeFromHeader. Keep the existing header-based 429
coverage and assert the previously failing request eventually completes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/ingestion/connectors/ai/claude-admin/connector.yaml`:
- Around line 23-48: Add a regression test using the visible reliability helper
or equivalent fixture to return HTTP 429 without a Retry-After header, then
verify the connector recovers successfully through the
ExponentialBackoffStrategy fallback configured alongside WaitTimeFromHeader.
Keep the existing header-based 429 coverage and assert the previously failing
request eventually completes.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 6f6d0823-370e-4c47-b914-e2ac619d6fd6

📥 Commits

Reviewing files that changed from the base of the PR and between 6cbf76a9ff788964a7770e31ca04ba207793bea2 and 428a2d4.

📒 Files selected for processing (2)
  • src/ingestion/connectors/ai/claude-admin/connector.yaml
  • src/ingestion/connectors/ai/claude-admin/descriptor.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/ingestion/connectors/ai/claude-admin/descriptor.yaml

@cyberdima
cyberdima merged commit cc04cfd into main Jul 28, 2026
42 checks passed
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.

claude-admin connector: offset-based pagination never terminates on Anthropic Admin API list endpoints

2 participants