fix: Stop retry transport retrying rate limited requests

[stevehipwell]

Related to #3565

---

Before the change?

• The new client retry logic incorrectly handled 429 responses
• The rate limit transport was before the throttler which could result in rate limit bypass (which would compound the retry issue)

After the change?

• The new client retry logic ignores 429 responses
• The throttler now fully respects the rate limits

Pull request checklist

• Schema migrations have been created if needed (example)
• Tests for the changes have been added (for bug fixes / features)
• Docs have been reviewed and added / updated if needed (for bug fixes / features)

Does this introduce a breaking change?

Please see our docs on breaking changes to help!

• Yes
• No

---

[github-actions]

👋 Hi, and thank you for this contribution!

This repo is maintained by GitHub and community members on a best-effort basis. We'll get to this as soon as we can.

You can help us prioritize by joining the discussion on open issues and PRs, sharing details on the changes you need, and reviewing other contributions.

---

_{🤖 This is an automated message.}

[Copilot]

Copilot review overview

🟡 Changes recommended

The error assertion is inverted, and the transport-order regression lacks combined coverage.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1 · 1

Open (2)

• Fix inverted error assertion in handles_error test · New
• Add transport test covering retry and semaphore ordering · New

What changed in this PR

These provider review instructions are being used.

This PR corrects rate-limit handling in the GitHub HTTP transport.

Changes:

• Excludes HTTP 429 responses from generic retries.
• Reorders rate-limit and throttling transports.
• Extracts callbacks and adds retry-policy tests.

File	Description
internal/​ghclient/​transport.go	Reorders transports and installs the retry policy.
internal/​ghclient/​retry.go	Adds the 429-aware retry policy.
internal/​ghclient/​retry_test.go	Tests retry-policy behavior.
internal/​ghclient/​ratelimit.go	Houses rate-limit logging callbacks.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Copilot review overview

🔵 Needs a closer look

The transport ordering lacks an integration-level regression test.

Review effort: Balanced
Findings: None

Resolved since last review (2)

• Fix inverted error assertion in handles_error test
• Add transport test covering retry and semaphore ordering

[Copilot]

Copilot review overview

🟡 Changes recommended

Zero-delay configurations regress, empty cache paths bypass validation, and exhausted retries leak response bodies.

Review effort: Balanced
Findings: 1 · 2

Open (3)

• Close final response body when retries are exhausted · New
• Validate BasePath before joining optional ref · New
• Preserve zero-delay retry configuration · New

[Copilot]

Copilot review overview

🟡 Changes recommended

Zero-delay configuration, disabled-retry body replay, and portions of the prior 5xx retry behavior currently regress.

Review effort: Balanced
Findings: 1 · 3 · 1

Open (5)

• Support request-body replay when generic retries are disabled · New
• Get defaulted retry delay with Get instead of GetOk · New
• Preserve broad 5xx retries while excluding 501 and 429 · New
• Preserve zero-delay retry configuration
• Test rate-limit retries remain constrained by the throttling semaphore · New

Resolved since last review (2)

• Close final response body when retries are exhausted
• Validate BasePath before joining optional ref

[Copilot]

Copilot review overview

🟡 Changes recommended

The shared stateful backoff breaks retry limits across requests, and the zero-delay compatibility path lacks regression coverage.

Review effort: Balanced
Findings: 1 · 1

Open (2)

• Create a fresh backoff chain for each request · New
• Test zero-delay retry configuration compatibility · New

Resolved since last review (5)

• Support request-body replay when generic retries are disabled
• Preserve broad 5xx retries while excluding 501 and 429
• Get defaulted retry delay with Get instead of GetOk
• Preserve zero-delay retry configuration
• Test rate-limit retries remain constrained by the throttling semaphore

[Copilot]

Copilot review overview

🔵 Needs a closer look

The zero-delay compatibility workaround needs a provider-level regression test.

Review effort: Balanced
Findings: None

Resolved since last review (2)

• Create a fresh backoff chain for each request
• Test zero-delay retry configuration compatibility

[Copilot]

Copilot review overview

🟡 Changes recommended

Retried POST and GraphQL requests can send an empty body after the first attempt.

Review effort: Balanced
Findings: 1

Open (1)

• Reset request body before each retry attempt · New

[robert-crandall]

Thanks for working through the retry and rate-limit changes!