Conversation
Collaborator
|
I think this needs to be otherwise updated with |
Author
Caught this, should be using parse_retry_config() now |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements CLI-66:
retry-on-forbiddensupport in the retries configuration, mirroring the existingretry-on-unavailablebehavior. cecli can now optionally retry API calls that fail with 403 Forbidden /PermissionDeniedErrorwhen the user explicitly opts in.This branch also carries the CLI-65 retry-config groundwork it builds on (public
parse_retry_config, retry config plumbing inmodels.py/base_coder.py, and the retry config test suite).Motivation / Problem Statement
Some providers return transient 403s (e.g., temporary permission propagation delays, gateway/edge authorization hiccups, provider-side quarantine flaps). Previously,
PermissionDeniedErrorwas hard-coded as non-retryable (retry: Falseinexceptions.py), so a transient 403 immediately aborted the request. Users asked for an opt-in way to retry these, symmetric withretry-on-unavailable.Design decision:
retry-on-forbiddendefaults toFalse. A 403 usually indicates an auth/permissions problem that retries cannot fix, so this is deliberately opt-in rather than default-on.What Changed
retry-on-forbiddenimplementation (CLI-66)cecli/models.pyModelSettings: new fieldretry_on_forbidden: bool = False.parse_retry_config(): parsesretry_on_forbidden/retry-on-forbidden(both key styles supported, defaultFalse) and documents it in the docstring.Model.send_completion(): readsretry-on-forbiddenfrom the retry config intoself.retry_on_forbidden, and adds aPermissionDeniedErrorbranch that ORs it intoshould_retry— same pattern asServiceUnavailableError→retry_on_unavailable.Model.simple_send_with_retries(): same handling using the localretry_on_forbiddenvalue.cecli/coders/base_coder.pyCoder.send_message()exception handler: addsif ex_info.name == "PermissionDeniedError": should_retry = should_retry or retry_config["retry_on_forbidden"]so chat requests honor the setting with the same backoff/timeout logic as other retriable errors.CLI-65 groundwork included in this branch
parse_retry_configmade public and used consistently acrossmodels.pyandbase_coder.py.tests/basic/test_retry_config.py: unit tests for retry config parsing (hyphenated/underscored keys, defaults, dict vs JSON-string inputs) and retry backoff timeout behavior insimple_send_with_retries.Configuration
Behavior
retry-on-forbidden: false(default): 403 /PermissionDeniedErrorfails immediately, exactly as before — no behavior change for existing users.retry-on-forbidden: true: 403s retry with the configured backoff factor andretry-timeoutcap, interruptible like other retries, and honor server-suggested delays from_extract_retry_delaywhen present.Testing
tests/basic/test_retry_config.pycovers config parsing and retry-loop backoff/timeout behavior (test_simple_send_with_retries_honors_timeout).actruns of.github/workflows/pre-commit.ymlfail with "Could not find any stages to run" — an infrastructure/workflow-config issue (no matching jobs), not a code lint violation.Notes / Follow-ups
cecli/website/docs/config/retries.mdis tracked in the plan (technical_writer scope) and can follow in a docs pass.cecli/sessions.pyand requirements pin adjustments relative tov1.6.2; these were introduced by the preceding CLI-65 commits on this branch.