Improve GitHub rate-limit handling in Oz PR review flow (REMOTE-2012)#484
Draft
seemeroland wants to merge 1 commit into
Draft
Improve GitHub rate-limit handling in Oz PR review flow (REMOTE-2012)#484seemeroland wants to merge 1 commit into
seemeroland wants to merge 1 commit into
Conversation
When GitHub rate-limits the Oz agent during review context gathering (e.g. while fetching the PR diff or checking the local skill file), the webhook handler previously propagated the exception as a silent 500 with no user-visible feedback. The /oz-review retry appeared to 'do nothing' because no new progress comment was created. Changes: - Catch exceptions from gather_review_context in ReviewWorkflow.build_dispatch. When any exception occurs (including rate-limit errors), post a user-visible comment on the PR explaining what happened and asking the user to retry with /oz-review, then return None to skip dispatch gracefully. Rate-limit errors (HTTP 429, RateLimitExceededException, or 403 with a rate-limit message body) get a specific message mentioning rate limits. - Add a retrigger hint to review run failure messages. When a review run fails in the cloud agent (e.g. due to a secondary rate limit loading the review skill), the error comment now includes 'Comment /oz-review on this pull request to try again.' so users know how to recover. - Add error_retrigger_hint property to ReviewWorkflow. The failure handler in handlers_for_workflow reads this attribute and passes it to report_error(), threading the hint through WorkflowProgressComment. Non-review workflows pass an empty hint (no change to their behavior). - Fix RETRIGGER_HINT wording. It previously said 'up to 3 times on the same pull request' which disagreed with MAX_DAILY_REVIEW_INVOCATIONS=5. Updated to 'Comment /oz-review on this pull request to request another review.' (no hardcoded count, always accurate). - Fix pre-existing test environment issues. The handler tests were failing because oz.helpers was stubbed without ENFORCEMENT_COMMENT_RUN_ID (used by core/workflows/__init__.py at module level). Added the constant to the stub. Similarly, added oz.ownership to the BuildReviewRequestTest stub registry to prevent cascading real-module imports that fail when oz.triage tries to import from the stubbed oz.helpers. Tests: 181 tests pass across test_builders.py, test_handlers.py, test_dispatch.py, test_routing.py, test_webhook_dispatch.py, test_poll_runs.py, test_workflow_adapters.py and related test files. Co-Authored-By: Oz <oz-agent@warp.dev>
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
Fixes two issues related to GitHub rate-limiting in the Oz PR review flow (REMOTE-2012):
Issue 1: Silent webhook failure when rate-limited during context gathering
When
gather_review_contextraises (e.g. due to GitHub API rate-limiting while fetching the PR diff or the local companion skill), the exception previously propagated throughevaluate_routeand the webhook handler returned a 500 with no user-visible feedback. The/oz-reviewretry appeared to "do nothing" because no new bot comment was ever created.Fix: Wrap
gather_review_contextinReviewWorkflow.build_dispatchwith try/except. When any exception occurs, post a user-visible comment on the PR and returnNoneto skip dispatch gracefully. Rate-limit errors (HTTP 429 /RateLimitExceededException/ 403 with rate-limit body) get a specific message; other GitHub API errors get a generic message. Both mention/oz-reviewto retry.Issue 2: Generic "unexpected error" message with no recovery guidance
When a review run fails in the cloud agent (e.g. because GitHub rate-limits the Oz platform while loading the review skill), the error comment said "I ran into an unexpected error while working on this." with no hint on how to recover.
Fix:
error_retrigger_hintproperty toReviewWorkflow. The failure handler inhandlers_for_workflowreads this attribute and passes it toreport_error(), which appends "Comment/oz-reviewon this pull request to try again." to the error message.retrigger_hint: str = ""parameter toWorkflowProgressComment.report_error()so any workflow can include a recovery hint.Bonus: Fix incorrect RETRIGGER_HINT count
The
RETRIGGER_HINTconstant said "up to 3 times on the same pull request" but the actual per-PR daily limit (MAX_DAILY_REVIEW_INVOCATIONS) is 5. Updated the hint text to not hardcode a count.Also: Fix pre-existing test environment issues
The handler and builder tests were broken in our test environment because:
oz.helperswas stubbed in_HandlerTestBasewithoutENFORCEMENT_COMMENT_RUN_ID(imported at module level bycore/workflows/__init__.py).oz.ownershipwas not stubbed inBuildReviewRequestTest, causing cascading real imports that hit the stubbedoz.helpers.Both fixed by extending the stub setup.
Conversation: https://staging.warp.dev/conversation/6eb5dc33-37b4-4466-b73d-50e841722115
Run: https://oz.staging.warp.dev/runs/019efb06-4d0e-7cf9-9dbb-cf88feac7c8f
This PR was generated with Oz.