Skip to content

fix: Prevent duplicate follow-up issues and handle rate limits #726

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
stranske merged 2 commits into from
Jan 10, 2026
Merged

fix: Prevent duplicate follow-up issues and handle rate limits #726

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4ca854f
fix: Prevent duplicate follow-up issues and handle rate limits in con…
stranske Jan 10, 2026
b75e78a
fix: update test expected name to match deprecated workflow
stranske Jan 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions .github/workflows/agents-verify-to-issue.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
name: Create Issue from Verification
name: Create Issue from Verification (DEPRECATED)

# Creates a follow-up issue from verification feedback when user adds verify:create-issue label
# This is a user-triggered workflow (not automatic) to avoid aggressive issue creation
# DEPRECATED: Use agents-verify-to-issue-v2.yml instead
# This workflow is disabled to prevent duplicate issue creation.
# The v2 version creates well-structured issues with tasks and acceptance criteria.

on:
pull_request_target:
Expand All @@ -14,7 +15,8 @@ permissions:

jobs:
create-issue:
if: github.event.label.name == 'verify:create-issue'
# DISABLED: v2 workflow handles this now to prevent duplicate issues
if: false && github.event.label.name == 'verify:create-issue'
runs-on: ubuntu-latest
steps:
- name: Check PR is merged
Expand Down
12 changes: 11 additions & 1 deletion .github/workflows/reusable-agents-verifier.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,17 @@ jobs:
script: |
const { buildVerifierContext } = require('./.workflows-lib/.github/scripts/agents_verifier_context.js');
const ciWorkflows = process.env.CI_WORKFLOWS || '[]';
await buildVerifierContext({ github, context, core, ciWorkflows });
try {
await buildVerifierContext({ github, context, core, ciWorkflows });
} catch (err) {
if (err.message && err.message.includes('rate limit')) {
core.warning(`Rate limited while building context: ${err.message}`);
Comment on lines +179 to +180

Copilot AI Jan 10, 2026

Copy link

Choose a reason for hiding this comment

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

The rate limit detection logic is too broad and may catch unrelated errors. The check err.message.includes('rate limit') will match any error message containing these words, including potentially unrelated errors. Based on the codebase's error handling patterns (found in api-helpers.js and error_classifier.js), rate limit errors typically have more specific characteristics such as HTTP status codes (403, 429) and specific error messages from GitHub's API. Consider checking for more specific patterns like checking the error status code or using a more precise message pattern.

Suggested change
if (err.message && err.message.includes('rate limit')) {
core.warning(`Rate limited while building context: ${err.message}`);
const status = (err && (err.status || (err.response && err.response.status))) || undefined;
const message = String((err && err.message) || '');
const isRateLimitError =
status === 403 ||
status === 429 ||
/API rate limit exceeded/i.test(message) ||
/secondary rate limit/i.test(message);
if (isRateLimitError) {
core.warning(`Rate limited while building context: ${message}`);

Copilot uses AI. Check for mistakes.
core.setOutput('should_run', 'false');
core.setOutput('skip_reason', 'Rate limited - please retry later');
} else {
throw err;
Comment on lines +179 to +184

Copilot AI Jan 10, 2026

Copy link

Choose a reason for hiding this comment

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

The error handler only catches and handles rate limit errors gracefully, but re-throws all other errors. However, based on examining the buildVerifierContext function, there are several API calls that could fail for various reasons (e.g., fetchClosingIssues with GraphQL, github.rest.pulls.get, queryVerifierCiResults). These API calls already have their own error handling that returns graceful fallbacks (empty arrays or empty strings). When any unexpected error occurs from buildVerifierContext that isn't a rate limit, it will be re-thrown and cause the workflow to fail. Consider whether other transient errors should also be handled gracefully, or document why only rate limit errors receive special treatment.

Suggested change
if (err.message && err.message.includes('rate limit')) {
core.warning(`Rate limited while building context: ${err.message}`);
core.setOutput('should_run', 'false');
core.setOutput('skip_reason', 'Rate limited - please retry later');
} else {
throw err;
if (err && err.message && err.message.includes('rate limit')) {
core.warning(`Rate limited while building context: ${err.message}`);
core.setOutput('should_run', 'false');
core.setOutput('skip_reason', 'Rate limited - please retry later');
} else {
// Treat other unexpected/transient errors gracefully by skipping the verifier
const message = (err && err.message) ? err.message : String(err);
core.error(`Error while building verifier context: ${message}`);
if (err && err.stack) {
console.error(err.stack);
}
core.setOutput('should_run', 'false');
core.setOutput('skip_reason', `Error while building context: ${message}`);

Copilot uses AI. Check for mistakes.
}
}
Comment on lines +176 to +186

Copilot AI Jan 10, 2026

Copy link

Choose a reason for hiding this comment

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

The new rate limit handling logic in this workflow lacks test coverage. The codebase has comprehensive test coverage for rate limit handling in other areas (as evidenced by tests in error-classifier.test.js, github-api-retry.test.js, and keepalive-loop.test.js), but there are no tests verifying that this specific try-catch block correctly handles rate limit errors and sets the appropriate outputs. Consider adding tests to verify the behavior when a rate limit error is thrown by buildVerifierContext.

Copilot uses AI. Check for mistakes.

- name: Stop when verifier is skipped
if: steps.context.outputs.should_run != 'true'
Expand Down
Empty file modified scripts/langchain/integration_layer.py
100644 → 100755
View file
Open in desktop
Empty file.
2 changes: 1 addition & 1 deletion tests/workflows/test_workflow_naming.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ def test_workflow_display_names_are_unique():
"agents-64-verify-agent-assignment.yml": "Agents 64 Verify Agent Assignment",
"agents-issue-optimizer.yml": "Agents Issue Optimizer",
"agents-verifier.yml": "Agents Verifier",
"agents-verify-to-issue.yml": "Create Issue from Verification",
"agents-verify-to-issue.yml": "Create Issue from Verification (DEPRECATED)",
"agents-verify-to-issue-v2.yml": "Create Issue from Verification (Enhanced)",
"agents-weekly-metrics.yml": "agents-weekly-metrics",
"agents-70-orchestrator.yml": "Agents 70 Orchestrator",
Expand Down
Loading