Skip to content

Ashwin/codex/pr2 retry consistency#377

Open
AshwinRenjith wants to merge 3 commits intogoogleworkspace:mainfrom
AshwinRenjith:ashwin/codex/pr2-retry-consistency
Open

Ashwin/codex/pr2 retry consistency#377
AshwinRenjith wants to merge 3 commits intogoogleworkspace:mainfrom
AshwinRenjith:ashwin/codex/pr2-retry-consistency

Conversation

@AshwinRenjith
Copy link

@AshwinRenjith AshwinRenjith commented Mar 10, 2026

Harden retry behavior consistency for discovery-driven command execution by routing the generic executor path through shared retry handling and tightening backoff safety.

This fixes a reliability gap where core executor requests were sent once while some helper flows retried on 429.

Description

This PR fixes inconsistent retry behavior in the primary HTTP execution path.

What changed

  • Routed generic executor sends through shared retry handling (send_builder_with_retry) so discovery-driven commands are resilient to HTTP 429 throttling.
  • Added retry-safety gating in executor:
    • Retry is applied only to retry-safe HTTP methods (GET, HEAD, OPTIONS, PUT, DELETE).
    • Non-idempotent methods like POST are sent once (no replay on 429).
  • Hardened backoff behavior:
    • Added a cap for Retry-After to prevent unbounded server-controlled sleep durations.
    • Kept exponential fallback behavior for missing/invalid Retry-After.
  • Reduced helper fragmentation:
    • Updated Gmail +triage list call to use shared retry helper.
  • Added regression tests covering:
    • 429 retry success in client retry wrappers.
    • no retry on non-429 responses.
    • generic executor recovery from 429 -> 200.
    • no retry/replay for POST on 429.
    • retry delay cap logic.

Dry Run Output:

{
  "note": "N/A — this PR does not add a new feature/command or change Discovery request body shape; it only hardens retry behavior."
}

Why this is safe

  • Retry logic is centralized at the generic executor boundary.
  • Non-idempotent replay risk is explicitly constrained by method gating.
  • Backoff is bounded to avoid availability issues from malicious or malformed Retry-After values.

Checklist:

  • My code follows the AGENTS.md guidelines (no generated google-* crates).
  • I have run cargo fmt --all to format the code perfectly.
  • I have run cargo clippy -- -D warnings and resolved all warnings.
  • I have added tests that prove my fix is effective or that my feature works.
  • I have provided a Changeset file (e.g. via pnpx changeset) to document my changes.

@changeset-bot
Copy link

changeset-bot bot commented Mar 10, 2026

🦋 Changeset detected

Latest commit: c843c76

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@googleworkspace/cli Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@googleworkspace-bot googleworkspace-bot added area: http area: core Core CLI parsing, commands, error handling, utilities labels Mar 10, 2026
@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly enhances the robustness and consistency of HTTP request handling within the application by centralizing and refining retry logic. It addresses a critical reliability gap by ensuring that all discovery-driven command executions benefit from resilient retry behavior, particularly against rate limiting, while simultaneously preventing unsafe retries for non-idempotent operations and safeguarding against excessive server-mandated delays.

Highlights

  • Retry Consistency: Routed generic executor HTTP requests through a shared retry mechanism (send_builder_with_retry) to handle 429 (Too Many Requests) responses, aligning with existing resilient helper paths.
  • Retry Safety: Implemented method-based gating for retries, ensuring only idempotent HTTP methods (GET, HEAD, OPTIONS, PUT, DELETE) are retried, preventing unintended side effects for non-idempotent methods like POST.
  • Backoff Hardening: Introduced a maximum cap (MAX_RETRY_AFTER_SECS) for the Retry-After header value to prevent excessively long server-controlled delays, while maintaining exponential backoff for missing or invalid headers.
  • Helper Integration: Updated the Gmail +triage list call to utilize the centralized send_with_retry helper, reducing code fragmentation.
  • Comprehensive Testing: Added new regression tests to cover successful 429 retries, non-429 response handling, generic executor recovery, POST no-retry behavior, and retry delay clamping.
Changelog
  • .changeset/executor-retry-consistency.md
    • Added a new changeset file documenting the improved retry behavior for generic executor HTTP sends.
  • src/client.rs
    • Introduced MAX_RETRY_AFTER_SECS constant to cap retry delays.
    • Implemented retry_delay_secs function to calculate retry delay, incorporating Retry-After header and exponential backoff with a maximum cap.
    • Modified send_with_retry to use the new retry_delay_secs function.
    • Added send_builder_with_retry function for retrying reqwest::RequestBuilder instances, handling clonable requests.
    • Expanded test suite with retry_delay_secs clamping and exponential fallback tests, and send_builder_with_retry specific tests.
  • src/executor.rs
    • Added is_retry_safe_method utility function to identify idempotent HTTP methods.
    • Integrated send_builder_with_retry into execute_method, applying retry logic conditionally based on is_retry_safe_method.
    • Introduced spawn_response_server helper for testing HTTP responses.
    • Added new tokio::test cases for execute_method to verify 429 retries for GET and no retries for POST.
  • src/helpers/gmail/triage.rs
    • Updated the Gmail handle_triage function to use the crate::client::send_with_retry helper for listing messages.
Activity
  • No specific activity (comments, reviews, or progress updates) was provided in the context for this pull request.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Generative AI Prohibited Use Policy, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request effectively hardens the retry logic for API requests, making the CLI more resilient to throttling (HTTP 429 errors). The core changes, which route generic executor requests through a shared retry handler and add safety gates for non-idempotent methods like POST, are well-implemented and improve consistency. The addition of a cap on the Retry-After duration is a good defensive measure.

I've identified a couple of areas for improvement to enhance maintainability: one is a simplification of the retry logic in src/client.rs, and the other is to address duplicated test helper code between src/client.rs and src/executor.rs. These are detailed in the review comments.

Overall, this is a solid improvement to the tool's reliability.

@github-actions
Copy link
Contributor

/gemini review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: core Core CLI parsing, commands, error handling, utilities area: http

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants