-
-
Notifications
You must be signed in to change notification settings - Fork 88
[feature] Add CI failure bot for automated responses #524 #562
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.
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
base: master
Are you sure you want to change the base?
[feature] Add CI failure bot for automated responses #524 #562
Conversation
WalkthroughAdds a CI Failure Bot script (.github/scripts/ci_failure_bot.py), a GitHub Actions workflow (.github/workflows/ci-failure-bot.yml), developer and user documentation, and unit tests. The bot collects failed workflow run job logs (handles ZIP archives and truncation), optionally fetches PR diff and workflow YAML, queries Google Gemini for AI analysis with a static fallback on errors, and posts or updates a deduplicated comment on the associated pull request. The workflow triggers on failed "OpenWISP Utils CI Build" runs and excludes dependabot. Sequence Diagram(s)sequenceDiagram
autonumber
participant Actions as GitHub Actions
participant Bot as CI Failure Bot
participant GH as GitHub API
participant Gemini as Gemini API
participant PR as Pull Request
Actions->>Bot: workflow_run completed (env: WORKFLOW_RUN_ID, REPOSITORY, PR_NUMBER, GITHUB_TOKEN, GEMINI_API_KEY)
Bot->>GH: GET /actions/runs/{id}/jobs
GH-->>Bot: jobs metadata
Bot->>GH: GET workflow run logs archive
GH-->>Bot: logs archive
Bot->>Bot: extract failed jobs & failing steps (truncate logs)
Bot->>GH: GET workflow YAML
GH-->>Bot: workflow YAML
alt PR_NUMBER provided
Bot->>GH: GET PR diff (title, body, diff)
GH-->>Bot: diff, title, body
end
Bot->>Gemini: send structured context (logs, YAML, PR diff, repo info)
Gemini-->>Bot: analysis / remediation message
alt Gemini error or failure
Bot->>Bot: generate fallback response
end
alt PR_NUMBER present
Bot->>GH: POST or UPDATE comment on PR
GH-->>PR: comment created/updated
else no PR
Bot-->Actions: log result, exit
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
Fix all issues with AI Agents 🤖
In @.github/scripts/ci_failure_bot.py:
- Around line 109-125: The method generate_commit_message_response is dead code
(never called); either remove it or call it from run() where other validations
happen: locate the run() method and after fetching the commit message (e.g.,
commit_message variable or the payload field you already parse), call
self.generate_commit_message_response(commit_message) and use its return value
to post a comment or append to the bot response flow, or simply delete the
generate_commit_message_response function if you don't want commit message
validation now; ensure you refer to the exact symbol
generate_commit_message_response when removing or invoking it and update any
tests/uses accordingly.
- Around line 134-136: The code currently builds comment_body with an automatic
mention via the variables author and comment_body and always calls
pr.create_issue_comment, which can spam the PR author; update this by removing
the "@{author}" mention or making mentions optional via a config flag (e.g.,
mention_author) and by adding rate-limiting/deduplication before calling
pr.create_issue_comment: record the last_mention timestamp per PR (in-memory
cache or persistent store) and only include the mention or post a comment if the
last mention is older than a configured cooldown, or skip adding the mention
entirely and post a non-mention comment instead; ensure you reference and modify
the creation site that constructs comment_body and the call to
pr.create_issue_comment to enforce the new logic.
In @.github/workflows/ci-failure-bot.yml:
- Around line 28-34: The step setting PR_NUMBER reads
github.event.workflow_run.pull_requests[0].number without checking for an empty
array; update the env for PR_NUMBER in the "Run CI Failure Bot" step to a
conditional expression that yields the PR number when pull_requests exists and
has items, otherwise an empty string (use a GitHub Actions expression checking
github.event.workflow_run.pull_requests and its length before indexing). This
prevents failures for workflow runs that have no associated PR while keeping the
Python script behavior unchanged.
🧹 Nitpick comments (4)
.github/scripts/ci_failure_bot.py (3)
18-20: Improve error message specificity for missing environment variables.The current error message "Missing env vars" doesn't indicate which variables are missing, making debugging harder.
🔎 Proposed fix for more descriptive error message
if not all([self.github_token, self.workflow_run_id, self.repository_name]): - print("Missing env vars") + missing = [] + if not self.github_token: missing.append('GITHUB_TOKEN') + if not self.workflow_run_id: missing.append('WORKFLOW_RUN_ID') + if not self.repository_name: missing.append('REPOSITORY') + print(f"Missing required environment variables: {', '.join(missing)}") sys.exit(1)
25-43: Refine exception handling for better error visibility.The broad
Exceptioncatch on line 41 can mask specific API errors. Consider catchingGithubExceptionspecifically or re-raising after logging.Based on static analysis hints, catching blind exceptions makes debugging harder.
🔎 Proposed fix for more specific exception handling
+from github import GithubException + class CIFailureBot: # ... existing code ... def get_workflow_logs(self): try: workflow_run = self.repo.get_workflow_run(int(self.workflow_run_id)) jobs = workflow_run.jobs() failed_jobs = [] for job in jobs: if job.conclusion == 'failure': for step in job.steps: if step.conclusion == 'failure': failed_jobs.append({ 'job_name': job.name, 'step_name': step.name, 'step_number': step.number }) return failed_jobs - except Exception as e: + except GithubException as e: print(f"Error getting logs: {e}") return []
127-139: Validate PR number and improve exception handling.Two concerns:
- Line 128 only checks truthiness of
pr_number; it should validate it's a non-empty string that can be converted to an integer.- Line 138 catches blind
Exception, which can mask specific API errors.🔎 Proposed fix for validation and exception handling
+from github import GithubException + class CIFailureBot: # ... existing code ... def post_comment(self, message): - if not self.pr_number: + if not self.pr_number or self.pr_number.strip() == '': print("No PR number, skipping comment") return try: + pr_num = int(self.pr_number) - pr = self.repo.get_pull(int(self.pr_number)) + pr = self.repo.get_pull(pr_num) author = pr.user.login comment_body = f"@{author} {message}" pr.create_issue_comment(comment_body) - print(f"Posted comment to PR #{self.pr_number}") + print(f"Posted comment to PR #{pr_num}") - except Exception as e: + except (ValueError, GithubException) as e: print(f"Error posting comment: {e}").github/workflows/ci-failure-bot.yml (1)
24-26: Pin dependency versions for reproducibility and stability in CI workflows.Installing packages without version constraints means each workflow run could install different versions, potentially introducing breaking changes. Consider pinning to specific stable versions.
🔎 Proposed fix with current stable versions
- name: Install dependencies run: | - pip install requests PyGithub + pip install requests==2.32.5 PyGithub==2.8.1
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/scripts/ci_failure_bot.py.github/workflows/ci-failure-bot.yml
🧰 Additional context used
🪛 Ruff (0.14.10)
.github/scripts/ci_failure_bot.py
40-40: Consider moving this statement to an else block
(TRY300)
41-41: Do not catch blind exception: Exception
(BLE001)
138-138: Do not catch blind exception: Exception
(BLE001)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
- GitHub Check: Python==3.11 | django~=5.2.0
- GitHub Check: Python==3.13 | django~=5.2.0
- GitHub Check: Python==3.11 | django~=4.2.0
- GitHub Check: Python==3.13 | django~=5.1.0
- GitHub Check: Python==3.12 | django~=5.2.0
- GitHub Check: Python==3.11 | django~=5.1.0
- GitHub Check: Python==3.11 | django~=5.0.0
- GitHub Check: Python==3.12 | django~=5.1.0
- GitHub Check: Python==3.12 | django~=4.2.0
- GitHub Check: Python==3.12 | django~=5.0.0
- GitHub Check: Python==3.10 | django~=4.2.0
- GitHub Check: Python==3.10 | django~=5.2.0
- GitHub Check: Python==3.10 | django~=5.1.0
- GitHub Check: Python==3.10 | django~=5.0.0
🔇 Additional comments (2)
.github/workflows/ci-failure-bot.yml (1)
1-13: LGTM! Workflow trigger and condition are properly configured.The workflow correctly triggers on the completion of the "OpenWISP Utils CI Build" workflow and only runs when the conclusion is 'failure'. The setup is appropriate for this use case.
.github/scripts/ci_failure_bot.py (1)
141-183: LGTM! The orchestration flow is well-structured.The
run()method properly orchestrates the workflow: retrieves logs, analyzes failure types, generates appropriate responses, includes a sensible fallback for unrecognized failures, and posts the final comment. The main execution block follows standard Python conventions.
4d35151 to
55627fb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Fix all issues with AI Agents 🤖
In @.github/scripts/ci_failure_bot.py:
- Line 9: The project imports github.Github in .github/scripts/ci_failure_bot.py
but PyGithub is not declared as a dependency; update the package manifest (e.g.,
setup.py) to add PyGithub (suggestion: 'PyGithub>=2.0.0') to the appropriate
extras_require/dev/ci dependency group so local installs and CI installs include
it, and ensure any CI/dev docs or requirements files reference the same version
constraint.
♻️ Duplicate comments (2)
.github/scripts/ci_failure_bot.py (2)
114-130: Unused method:generate_commit_message_responseis never called.This method is defined but not invoked anywhere in the
run()method. Either integrate commit message validation into the bot's flow or remove this dead code.Would you like me to suggest how to integrate commit message validation into the workflow, or would you prefer to remove this method for now?
139-141: Remove or make the automatic PR author mention optional and rate-limited.GitHub's automation policies explicitly forbid mass-mentioning and repeatedly pinging users. The current implementation automatically mentions the PR author in every CI failure comment (line 140), which could violate GitHub's anti-spam guidelines if failures occur frequently. Either remove the automatic @mention entirely or implement rate-limiting and deduplication to avoid repeated pings on the same PR.
🔎 Remove automatic mention
try: pr = self.repo.get_pull(int(self.pr_number)) - author = pr.user.login - comment_body = f"@{author} {message}" - pr.create_issue_comment(comment_body) + pr.create_issue_comment(message) print(f"Posted comment to PR #{self.pr_number}")
🧹 Nitpick comments (3)
.github/scripts/ci_failure_bot.py (3)
19-21: Improve error message to specify which environment variables are missing.The error message "Missing env vars" doesn't indicate which specific variables are missing, making debugging harder for users.
🔎 More descriptive error message
if not all([self.github_token, self.workflow_run_id, self.repository_name]): - print("Missing env vars") + missing = [] + if not self.github_token: + missing.append("GITHUB_TOKEN") + if not self.workflow_run_id: + missing.append("WORKFLOW_RUN_ID") + if not self.repository_name: + missing.append("REPOSITORY") + print(f"Missing required environment variables: {', '.join(missing)}") sys.exit(1)
44-46: Make exception handling more specific or enhance error logging.Catching
Exceptionis too broad and may hide important issues like authentication failures, network errors, or API rate limits. At minimum, log the exception type to aid debugging.🔎 Enhanced error logging
except Exception as e: - print(f"Error getting logs: {e}") + print(f"Error getting logs ({type(e).__name__}): {e}") return []Or, better yet, catch specific exceptions:
- except Exception as e: - print(f"Error getting logs: {e}") + except (GithubException, ValueError) as e: + print(f"Error getting workflow logs: {e}") return []Note: You'll need to import
GithubExceptionfromgithubif using the specific approach.
143-144: Enhance exception handling specificity.Similar to the
get_workflow_logsmethod, catching broadExceptioncan hide important API errors. Consider catching specific exceptions or improving error logging.🔎 More specific exception handling
- except Exception as e: - print(f"Error posting comment: {e}") + except (GithubException, ValueError) as e: + print(f"Error posting comment ({type(e).__name__}): {e}")Note: Import
GithubExceptionfromgithubif using this approach.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/scripts/ci_failure_bot.py.github/workflows/ci-failure-bot.yml
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/ci-failure-bot.yml
🧰 Additional context used
🪛 Ruff (0.14.10)
.github/scripts/ci_failure_bot.py
43-43: Consider moving this statement to an else block
(TRY300)
44-44: Do not catch blind exception: Exception
(BLE001)
143-143: Do not catch blind exception: Exception
(BLE001)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
- GitHub Check: Python==3.12 | django~=5.1.0
- GitHub Check: Python==3.13 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=5.2.0
- GitHub Check: Python==3.13 | django~=5.1.0
- GitHub Check: Python==3.10 | django~=5.0.0
- GitHub Check: Python==3.11 | django~=5.1.0
- GitHub Check: Python==3.12 | django~=5.0.0
- GitHub Check: Python==3.12 | django~=4.2.0
- GitHub Check: Python==3.10 | django~=5.2.0
- GitHub Check: Python==3.10 | django~=4.2.0
- GitHub Check: Python==3.11 | django~=4.2.0
- GitHub Check: Python==3.11 | django~=5.0.0
- GitHub Check: Python==3.11 | django~=5.2.0
- GitHub Check: Python==3.10 | django~=5.1.0
🔇 Additional comments (4)
.github/scripts/ci_failure_bot.py (4)
48-63: LGTM! Failure classification logic is sound.The method correctly categorizes failures based on step names with case-insensitive matching and proper deduplication.
65-112: LGTM! Response templates provide clear, actionable guidance.All three response generators (
generate_qa_response,generate_test_response,generate_setup_response) offer specific commands and helpful troubleshooting steps with appropriate documentation links.
146-185: LGTM! Main orchestration logic is well-structured.The
run()method properly coordinates workflow log retrieval, failure analysis, response generation, and comment posting. The fallback to a generic message for unrecognized failures ensures users always receive guidance.
188-190: LGTM! Standard entry point pattern.The script entry point follows Python best practices.
55627fb to
2baccea
Compare
|
Hi @nemesifier , Got the CI failure bot working! It catches QA failures, test issues, and setup problems, then posts helpful responses with the fix commands and links to contributing docs. Tested it on my own repo first like you suggested - triggers correctly when builds fail and generates the right responses. This covers the first part i mentioned. Let me know how it looks and I can work on the auto-assignment and stalled PR stuff next. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Fix all issues with AI Agents 🤖
In @.github/scripts/ci_failure_bot.py:
- Around line 114-124: post_comment currently always creates a new PR comment
which causes duplicates; modify post_comment to fetch existing PR comments via
pr.get_issue_comments(), determine the bot's username via
self.github.get_user().login (or self.github.login), scan comments for ones
authored by the bot and matching the failure signature (e.g., a stable substring
from message like "CI Build Failed" or exact message match), and if found either
update that comment with comment.edit(message) or skip creating a new one; only
call pr.create_issue_comment(message) when no matching bot comment exists and
keep existing try/except behavior for errors.
- Around line 26-46: The code calls int(self.workflow_run_id) in
get_workflow_logs which can raise ValueError if the env var is non-numeric; add
validation in the class __init__ (where self.workflow_run_id is set) to parse
and store it as an integer (or raise/exit with a clear message) and reject
non-integer values, and update get_workflow_logs to use the validated integer
attribute (e.g., self.workflow_run_id_int) instead of converting at call time;
ensure __init__ handles missing/invalid input gracefully so get_workflow_logs no
longer needs to guard against ValueError.
♻️ Duplicate comments (1)
.github/scripts/ci_failure_bot.py (1)
9-9: PyGithub dependency still needs to be added to project manifest.This was flagged in a previous review: the script imports PyGithub but it's not declared in the project's dependency manifest (e.g.,
setup.py). While the workflow installs it directly, it should be tracked in the project's dependencies for consistency and local development support.Based on past review comments.
🧹 Nitpick comments (1)
.github/scripts/ci_failure_bot.py (1)
100-112: Consider making version numbers configurable.Lines 108-109 hardcode Python and Django version numbers. As new versions are released or old ones deprecated, this will require manual script updates. Consider moving these to environment variables, a config file, or reading from the project's CI matrix configuration.
💡 Example: Use environment variables
def generate_setup_response(self): + python_versions = os.environ.get("SUPPORTED_PYTHON_VERSIONS", "3.10, 3.11, 3.12, 3.13") + django_versions = os.environ.get("SUPPORTED_DJANGO_VERSIONS", "4.2, 5.0, 5.1, 5.2") return f""" ## Setup Failed There was an issue with dependencies or environment setup. Check if you added new dependencies to requirements-test.txt. Verify Python/Django version compatibility: -- Python: 3.10, 3.11, 3.12, 3.13 -- Django: 4.2, 5.0, 5.1, 5.2 +- Python: {python_versions} +- Django: {django_versions} The CI will retry automatically, or push a small change to trigger rebuild. """
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/scripts/ci_failure_bot.py.github/workflows/ci-failure-bot.yml
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/ci-failure-bot.yml
🧰 Additional context used
🪛 Ruff (0.14.10)
.github/scripts/ci_failure_bot.py
43-43: Consider moving this statement to an else block
(TRY300)
44-44: Do not catch blind exception: Exception
(BLE001)
123-123: Do not catch blind exception: Exception
(BLE001)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
- GitHub Check: Python==3.13 | django~=5.1.0
- GitHub Check: Python==3.13 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=4.2.0
- GitHub Check: Python==3.11 | django~=4.2.0
- GitHub Check: Python==3.12 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=5.0.0
- GitHub Check: Python==3.11 | django~=5.1.0
- GitHub Check: Python==3.10 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=5.1.0
- GitHub Check: Python==3.11 | django~=5.2.0
- GitHub Check: Python==3.10 | django~=5.1.0
- GitHub Check: Python==3.11 | django~=5.0.0
- GitHub Check: Python==3.10 | django~=5.0.0
- GitHub Check: Python==3.10 | django~=4.2.0
🔇 Additional comments (5)
.github/scripts/ci_failure_bot.py (5)
48-63: LGTM!The failure classification logic is clear and handles the main categories appropriately. Using
set()to deduplicate failure types is a good approach.
65-80: LGTM!The QA response provides clear, actionable guidance with the appropriate command and documentation link.
82-98: LGTM!The test failure response provides helpful debugging steps and covers common issues appropriately.
126-165: LGTM!The orchestration logic is well-structured. The bot correctly handles multiple failure types, provides fallback messaging, and gracefully handles the case when no failure logs are found.
168-170: LGTM!Standard main execution pattern is correctly implemented.
2baccea to
8d3d6bd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (2)
.github/workflows/ci-failure-bot.yml (1)
28-34: The empty pull_requests array issue has been addressed.Line 33 correctly uses the
|| ''fallback to handle cases wherepull_requestsis empty (e.g., manual workflow runs or direct pushes). The Python script'spost_commentmethod gracefully handles an emptyPR_NUMBER..github/scripts/ci_failure_bot.py (1)
1-11: PyGithub dependency handling noted in previous review.The import of PyGithub (line 9) without declaring it in the project's
setup.pyhas already been flagged in a previous review. While the workflow explicitly installs it, adding it to the project's dependency manifest would improve local development support.
🧹 Nitpick comments (2)
.github/workflows/ci-failure-bot.yml (1)
24-26: Consider pinning dependency versions for reproducibility.The workflow installs
requestsandPyGithubwithout version constraints. While this ensures you get the latest versions, it could lead to unexpected breakage if major version changes introduce breaking changes.🔎 Example: Pin dependency versions
- name: Install dependencies run: | - pip install requests PyGithub + pip install requests>=2.31.0 PyGithub>=2.0.0.github/scripts/ci_failure_bot.py (1)
32-53: Consider catching more specific exceptions.Line 50 catches the broad
Exceptiontype. While this makes the bot resilient to API failures, catching more specific exceptions (e.g.,GithubException,RateLimitExceededException) would provide better error diagnosis and allow you to handle different failure modes appropriately.🔎 Example: Catch specific exceptions
from github import GithubException, RateLimitExceededException def get_workflow_logs(self): try: workflow_run = self.repo.get_workflow_run(self.workflow_run_id) jobs = workflow_run.jobs() # ... rest of logic except RateLimitExceededException as e: print(f"Rate limit exceeded: {e}") return [] except GithubException as e: print(f"GitHub API error: {e}") return []
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/scripts/ci_failure_bot.py.github/workflows/ci-failure-bot.yml
🧰 Additional context used
🪛 Ruff (0.14.10)
.github/scripts/ci_failure_bot.py
49-49: Consider moving this statement to an else block
(TRY300)
50-50: Do not catch blind exception: Exception
(BLE001)
144-144: Do not catch blind exception: Exception
(BLE001)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
- GitHub Check: Python==3.10 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=4.2.0
- GitHub Check: Python==3.11 | django~=4.2.0
- GitHub Check: Python==3.11 | django~=5.1.0
- GitHub Check: Python==3.10 | django~=5.1.0
- GitHub Check: Python==3.12 | django~=5.0.0
- GitHub Check: Python==3.13 | django~=5.1.0
- GitHub Check: Python==3.10 | django~=5.0.0
- GitHub Check: Python==3.12 | django~=5.2.0
- GitHub Check: Python==3.13 | django~=5.2.0
- GitHub Check: Python==3.11 | django~=5.2.0
- GitHub Check: Python==3.10 | django~=4.2.0
- GitHub Check: Python==3.11 | django~=5.0.0
- GitHub Check: Python==3.12 | django~=5.1.0
🔇 Additional comments (11)
.github/workflows/ci-failure-bot.yml (3)
1-9: LGTM: Workflow trigger configuration is correct.The
workflow_runtrigger on the "OpenWISP Utils CI Build" workflow with thecompletedevent type is the appropriate way to respond to CI failures. The additional failure check on line 13 ensures the bot only runs when needed.
10-17: LGTM: Job configuration and checkout step are appropriate.The failure condition and ubuntu-latest runner are correctly configured. Using
actions/checkout@v6is current best practice.
19-22: LGTM: Python setup is appropriate.Python 3.11 is a stable, well-supported version suitable for this automation script.
.github/scripts/ci_failure_bot.py (8)
12-31: LGTM: Input validation has been properly implemented.The
__init__method correctly validates that required environment variables are present (line 19) and thatworkflow_run_idis numeric (lines 23-27) before attempting to use it. This addresses the previous concern about potentialValueErrorexceptions during runtime.
54-69: LGTM: Failure type analysis logic is sound.The method correctly categorizes failures by analyzing step names with case-insensitive matching and returns unique failure types. The keyword-based classification appropriately handles QA checks, tests, and setup failures.
71-118: LGTM: Response generation methods provide helpful guidance.The response templates offer clear, actionable guidance for QA failures, test failures, and setup issues. The commands and links to contributing guidelines will help contributors resolve issues quickly.
Note: The Python and Django versions on lines 114-115 are hardcoded and may need periodic updates as new versions are released.
120-123: LGTM: Early return for missing PR number is appropriate.The check on line 121 correctly handles cases where the bot is triggered by workflow runs without an associated PR (e.g., direct pushes or manual runs).
128-146: Deduplication logic successfully addresses previous concerns.The implementation correctly checks for existing bot comments (lines 128-139) and updates them rather than creating duplicates. This addresses the major issue raised in a previous review.
The search strings on lines 133-136 ("CI Build Failed" and "QA Checks Failed") should continue to work as the fallback message on line 172 and the QA response on line 73 contain these markers.
147-187: LGTM: Main orchestration logic is well-structured.The
runmethod effectively coordinates log retrieval, failure analysis, response generation, and comment posting. The fallback message (lines 169-181) ensures helpful guidance is provided even when specific failure types aren't detected.
189-191: LGTM: Standard script execution pattern.The main execution block follows Python best practices.
124-146: Add validation before converting PR_NUMBER to int.Line 126 converts
self.pr_numbertoint()without validation. IfPR_NUMBERis an empty string (which can happen when the workflow fallback on line 33 of the YAML file triggers), this will raise aValueErrorand crash the bot. The early return on line 121 checks for falsy values, but an empty string passes the truthy check.🔎 Proposed fix to validate PR_NUMBER
def post_comment(self, message): - if not self.pr_number: + if not self.pr_number or self.pr_number == '': print("No PR number, skipping comment") return try: - pr = self.repo.get_pull(int(self.pr_number)) + try: + pr_num = int(self.pr_number) + except ValueError: + print(f"Invalid PR_NUMBER: {self.pr_number}") + return + + pr = self.repo.get_pull(pr_num)Likely an incorrect or invalid review comment.
Implements automated bot responses for common CI failures: - Parses QA check failures (flake8, black, isort) - Detects test failures and setup issues - Provides specific fix suggestions with links - Removes automatic mentions to prevent spam - Handles edge cases for workflow runs without PRs Tested on personal repository before submission. Closes openwisp#524
8d3d6bd to
939a127
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
.github/scripts/ci_failure_bot.py (1)
9-9: PyGithub dependency still not declared in project manifest.This was flagged in a previous review and remains unaddressed. The script imports
github.Githubbut PyGithub is not insetup.py. While the workflow installs it explicitly, adding it toextras_requireensures consistency for local development.
🧹 Nitpick comments (4)
.github/scripts/ci_failure_bot.py (4)
19-21: Consider logging which environment variables are missing.The current message "Missing env vars" doesn't indicate which specific variables are absent, making debugging harder when the workflow fails during initialization.
🔎 Suggested improvement
- if not all([self.github_token, self.workflow_run_id, self.repository_name]): - print("Missing env vars") + missing = [] + if not self.github_token: + missing.append("GITHUB_TOKEN") + if not self.workflow_run_id: + missing.append("WORKFLOW_RUN_ID") + if not self.repository_name: + missing.append("REPOSITORY") + if missing: + print(f"Missing required env vars: {', '.join(missing)}") sys.exit(1)
50-52: Avoid catching bareException.Catching
Exceptionwill swallow all errors includingKeyboardInterruptandSystemExit. Use more specific exception types from PyGithub (e.g.,GithubException) to handle API errors while allowing unexpected exceptions to propagate.🔎 Suggested fix
+from github import Github, GithubException + # In get_workflow_logs: - except Exception as e: - print(f"Error getting logs: {e}") + except GithubException as e: + print(f"Error getting workflow logs: {e}") return []
134-136: Cache the bot login to avoid repeated API calls.
self.github.get_user().loginmakes a GitHub API call each timepost_commentis invoked. Consider caching this value in__init__to reduce API usage and improve performance.🔎 Suggested approach
# In __init__, after line 30: + self.bot_login = self.github.get_user().login # In post_comment, replace line 135: - bot_login = self.github.get_user().login + bot_login = self.bot_login
150-151: Use specific exception types instead of bareException.Similar to
get_workflow_logs, this catch-all will swallow unexpected errors. Consider catchingGithubExceptionfor API errors andValueErrorfor the PR number conversion (which is already handled separately).🔎 Suggested fix
- except Exception as e: + except GithubException as e: print(f"Error posting comment: {e}")
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/scripts/ci_failure_bot.py.github/workflows/ci-failure-bot.yml
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/ci-failure-bot.yml
🧰 Additional context used
🪛 Ruff (0.14.10)
.github/scripts/ci_failure_bot.py
49-49: Consider moving this statement to an else block
(TRY300)
50-50: Do not catch blind exception: Exception
(BLE001)
150-150: Do not catch blind exception: Exception
(BLE001)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
- GitHub Check: Python==3.10 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=5.2.0
- GitHub Check: Python==3.13 | django~=5.1.0
- GitHub Check: Python==3.11 | django~=5.0.0
- GitHub Check: Python==3.13 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=5.0.0
- GitHub Check: Python==3.12 | django~=5.1.0
- GitHub Check: Python==3.11 | django~=5.1.0
- GitHub Check: Python==3.10 | django~=4.2.0
- GitHub Check: Python==3.10 | django~=5.1.0
- GitHub Check: Python==3.11 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=4.2.0
- GitHub Check: Python==3.10 | django~=5.0.0
- GitHub Check: Python==3.11 | django~=4.2.0
🔇 Additional comments (4)
.github/scripts/ci_failure_bot.py (4)
54-69: LGTM!The failure type classification logic is clear and the deduplication using
set()is appropriate.
71-118: LGTM!The response templates are clear, actionable, and provide helpful links to documentation.
153-192: LGTM!The orchestration logic is clean with appropriate fallback handling for unclassified failures.
195-197: LGTM!Standard entry point pattern.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not seeing any response in Srinath0916/ci-bot-test#1. Is it really working?
Would this work as a reusable github actions?
Docs and tests are completely missing but I guess we can spend time on that once we are sure this is going in the right direction.
PS: please let's exclude dependabot PRs.
.github/workflows/ci-failure-bot.yml
Outdated
| WORKFLOW_RUN_ID: ${{ github.event.workflow_run.id }} | ||
| REPOSITORY: ${{ github.repository }} | ||
| PR_NUMBER: ${{ github.event.workflow_run.pull_requests[0].number || '' }} | ||
| run: python .github/scripts/ci_failure_bot.py |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would we be able to call the same logic from other repositories in the openwisp org?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @nemesifier , for pointing this out, I should have clarified this earlier.
I double-checked the workflow logs: the bot is triggering correctly, detecting the failure type, and attempting to post a comment. In my personal test repository, the comment fails with a 403 Resource not accessible by integration, which is a known limitation when using workflow_run + GITHUB_TOKEN on personal repos. The workflow log reflects this.
In OpenWISP organization repositories, where the token has issue/PR write permissions, this limitation should not apply.
Regarding reuse: Yes, the logic can be made reusable across OpenWISP repositories by parameterizing repository-specific context (repo name, URLs, workflow name). Other repositories would only need to add the workflow file and rely on org-level secrets.
I’ll also exclude Dependabot PRs at both the workflow and script level.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please document usage, follow the current structure: https://openwisp.io/docs/dev/utils/developer/reusable-github-utils.html
.github/scripts/ci_failure_bot.py
Outdated
| Check the CI logs above for specific error details. Common issues: | ||
| - Import errors from missing dependencies | ||
| - Logic changes that broke existing functionality | ||
| - Missing test dependencies |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A static message cannot cover the complexity of our incoming PRs. We need a dynamic first line of defense against the current avalanche of contributions.
We are wasting valuable maintainer time explaining basic mistakes that an LLM can identify. Since we have a paid Gemini API token with sufficient capacity, we will use it to analyze build failures and outputs. By feeding Gemini the build logs, the YAML workflow, and the repository context, it will generate tailored feedback that provides immediate, actionable steps for the contributor.
I am opting for a direct and honest communication style. The goal is to provide high-quality technical feedback while maintaining a firm standard; we will not be overly polite to those submitting low-effort or spam contributions. This ensures we save maintainer energy and only engage deeply with PRs that meet our baseline requirements.
Structured Gemini System Prompt
Here's a prompt example:
### ROLE
You are the "Automated Maintainer Gatekeeper." Your goal is to analyze Pull Request (PR) build failures and provide direct, technically accurate, and no-nonsense feedback to contributors.
### INPUT CONTEXT PROVIDED
1. **Build Output/Logs:** The specific error traces from the failed CI run.
2. **YAML Workflow:** The CI configuration defining how tests are executed.
3. **PR Diff:** The actual code changes submitted.
4. **Project Name:** [Name]
5. **Repository:** [Insert repo URL].
6. **run-qa-checks:** [Insert URL of run-qa-checks script]
7. **runtests:** [Insert URL of runtests script]
### TASK
Analyze the provided context to determine why the build failed. Categorize the failure and respond according to the "Tone Guidelines" below.
### TONE GUIDELINES
- **Direct & Honest:** Do not use "fluff" or overly polite corporate language.
- **Firm Standards:** If a PR is low-effort, spammy, or fails to follow basic instructions, state that clearly.
- **Action-Oriented:** Provide the exact command or file change needed to fix the error, unless the PR is spammy, in which case we should just declare the PR as potential SPAM and ask maintainers to manually review it.
### RESPONSE STRUCTURE
1. **Status Summary:** A one-sentence blunt assessment of the failure.
2. **Technical Diagnosis:** - Identify the specific line/test that failed.
- Explain *why* it failed.
3. **Required Action:** Provide a code block or specific steps the contributor must take.
4. **Quality Warning (If Applicable):** If the PR appears to be "spam" (e.g., trivial README changes, AI-generated nonsense, or repeated basic errors), include a firm statement that such contributions are a drain on project resources and ping the maintainers asking them for manual review.
### EXAMPLE RESPONSE STYLE
"The build failed because you neglected to update the test suite to match your logic changes. This project does not accept functional changes without corresponding test updates. Refer to the log at line 452. Update `tests/logic_test.py` before re-submitting. We prioritize high-quality, ready-to-merge code; please ensure you run local tests before pushing."There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right @nemesifier , static responses won't scale for the kind of PRs we're getting.
I understand the direction now: use Gemini to analyze real build logs, diffs, and workflow context and generate targeted feedback, rather than relying on predefined messages.
I'll pivot the implementation to this Gemini-based approach for intelligent failure analysis. This will provide the dynamic, context-aware responses that can actually handle the complexity of real PRs.
.github/scripts/ci_failure_bot.py
Outdated
| - Python: 3.10, 3.11, 3.12, 3.13 | ||
| - Django: 4.2, 5.0, 5.1, 5.2 | ||
|
|
||
| The CI will retry automatically, or push a small change to trigger rebuild. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand the point of this. Given my previous comment, do you think this is still needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This static message approach isn't needed given the Gemini integration direction.
The static responses were just to validate the workflow mechanics, but the AI analysis will provide much better, context-specific guidance.
I'll focus on the Gemini approach instead.
|
Hi @nemesifier , I’ve implemented a Gemini-based version locally following the approach and system prompt you outlined (using build logs, PR diff, workflow context, and a direct tone). Before pushing it, I wanted to check if you’re happy with proceeding in this direction and with introducing a GEMINI_API_KEY dependency at the org level. Let me know and I’ll move forward accordingly. |
nemesifier
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Srinath0916 before merging it here we need to test this somewhere else.
I do not see any output in Srinath0916/ci-bot-test#1, how are you testing this?
|
Hi @nemesifier , I can show you the working bot demo here: srinath-testing/gemini-ci-bot-demo#1 You can see the bot posted: "🤖 Demo CI Failure Bot (AI-powered)" with structured analysis. The reason you don't see output in Srinath0916/ci-bot-test#1 is that personal repos have token permission limitations with workflow_run triggers. The demo repo I created specifically for this has the right setup. The Gemini integration is ready locally - I can push it once you confirm the demo proves the concept works. Would you like me to proceed with the full AI implementation? |
nemesifier
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's encouraging @Srinath0916, please push your progress and then focus on adding automated tests, docs and following up reviews.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🤖 Fix all issues with AI Agents
In @.github/scripts/ci_failure_bot.py:
- Line 71: The requests.get call using variables logs_url and headers has no
timeout and can hang; modify the call to pass a timeout (e.g., timeout=10) so it
becomes requests.get(logs_url, headers=headers, timeout=<seconds>), and add a
surrounding try/except to catch requests.exceptions.Timeout (or
requests.exceptions.RequestException) to log the timeout/error and handle
failure gracefully where response is expected.
- Around line 113-123: requests.get call may hang and the except catches all
exceptions; add a timeout argument to requests.get (e.g., requests.get(diff_url,
headers=headers, timeout=10)) and replace the broad except Exception with except
requests.RequestException as e to only catch network-related errors, then log or
print the error variable as before.
- Around line 16-53: The constructor currently hardcodes self.model =
genai.GenerativeModel("gemini-1.5-flash") which is deprecated; update __init__
to read a GEMINI_MODEL env var (e.g., model_name =
os.environ.get("GEMINI_MODEL", "gemini-2.5-flash")) and pass that into
genai.GenerativeModel so the model can be changed without code edits, and ensure
genai.configure(api_key=...) remains before instantiating self.model; optionally
validate that model_name is non-empty and include its variable name in error
messages.
🧹 Nitpick comments (3)
.github/scripts/ci_failure_bot.py (3)
134-136: Narrow the exception type for better debuggability.Catching bare
Exceptioncan mask unexpected errors. Consider catchingGithubExceptionspecifically.🔎 Proposed fix
- except Exception as e: + except GithubException as e: print(f"Error getting workflow YAML: {e}") return None
154-188: Remove ineffective# noqacomments from within the string literal.The
# noqa: E501comments on lines 154, 157, 169, 173-175, 182, 185 are inside the multi-line string and have no effect on linter behavior. They just clutter the prompt sent to Gemini.🔎 Proposed fix
Remove all
# noqa: E501suffixes from within the context string. If you need to suppress E501 for this file, add a single# noqa: E501comment on line 155 (the opening of the f-string), or configure the linter to ignore this file/section.- # noqa: E501 (ignore line length for multiline string) context = f""" ### ROLE -You are the "Automated Maintainer Gatekeeper." Your goal is to analyze Pull Request (PR) build failures and provide direct, technically accurate, and no-nonsense feedback to contributors. # noqa: E501 +You are the "Automated Maintainer Gatekeeper." Your goal is to analyze Pull Request (PR) build failures and provide direct, technically accurate, and no-nonsense feedback to contributors.Apply similar cleanup to lines 169, 173, 174, 175, 182, and 185.
246-253: Narrow exception handling for actor check.The dependabot check catches bare
Exception. Since this is a non-critical check (bot continues anyway), consider catchingGithubExceptionspecifically or at minimumAttributeErrorfor the case whereactormight be None.🔎 Proposed fix
try: workflow_run = self.repo.get_workflow_run(self.workflow_run_id) - actor_login = workflow_run.actor.login.lower() - if "dependabot" in actor_login: + if workflow_run.actor and "dependabot" in workflow_run.actor.login.lower(): print(f"Skipping dependabot PR from {workflow_run.actor.login}") return - except Exception as e: + except (GithubException, AttributeError) as e: print(f"Warning: Could not check actor: {e}")
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/scripts/ci_failure_bot.py.github/workflows/ci-failure-bot.yml
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/ci-failure-bot.yml
🧰 Additional context used
🪛 Ruff (0.14.10)
.github/scripts/ci_failure_bot.py
71-71: Probable use of requests call without timeout
(S113)
93-93: Consider moving this statement to an else block
(TRY300)
113-113: Probable use of requests call without timeout
(S113)
120-120: Do not catch blind exception: Exception
(BLE001)
134-134: Do not catch blind exception: Exception
(BLE001)
154-154: Unused noqa directive (non-enabled: E501)
Remove unused noqa directive
(RUF100)
192-192: Consider moving this statement to an else block
(TRY300)
193-193: Do not catch blind exception: Exception
(BLE001)
252-252: Do not catch blind exception: Exception
(BLE001)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
- GitHub Check: Python==3.10 | django~=4.2.0
- GitHub Check: Python==3.10 | django~=5.1.0
- GitHub Check: Python==3.13 | django~=5.1.0
- GitHub Check: Python==3.10 | django~=5.0.0
- GitHub Check: Python==3.11 | django~=4.2.0
- GitHub Check: Python==3.11 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=5.0.0
- GitHub Check: Python==3.13 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=5.1.0
- GitHub Check: Python==3.11 | django~=5.0.0
- GitHub Check: Python==3.11 | django~=5.1.0
- GitHub Check: Python==3.12 | django~=4.2.0
- GitHub Check: Python==3.12 | django~=5.2.0
- GitHub Check: Python==3.10 | django~=5.2.0
🔇 Additional comments (4)
.github/scripts/ci_failure_bot.py (4)
1-13: Imports and setup look appropriate.The imports are well-organized and appropriate for the script's functionality. Note that a past review flagged adding
PyGithubandgoogle-generativeaito the project's dependency manifest for local development support—ensure this is tracked.
212-239: Comment deduplication is well-implemented.The logic correctly checks for existing bot comments and updates them rather than creating duplicates. This addresses previous review feedback about preventing comment spam.
197-210: Fallback response is appropriate.The static fallback provides useful guidance when Gemini is unavailable, with actionable commands and a link to documentation.
76-78: Token limits are not a constraint for these truncations.Gemini 1.5 Flash has a 1,048,576-token context window (~4 million characters). With logs truncated to 5000 chars and diff to 8000 chars, plus the prompt template, the total message size (~15-20K characters) is negligible. The truncations are sensible for practical reasons—API response speed, cost optimization, and filtering noise—not because of Gemini's context limitations.
294b7f6 to
2e7b8f1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI Agents
In @.github/workflows/ci-failure-bot.yml:
- Around line 29-32: The comment "Pin dependency versions for reproducibility"
is inaccurate relative to the pip install line under the "Install dependencies"
step; either change the version specifiers in the pip install command to exact
pins using == (e.g., requests==2.31.0 PyGithub==2.0.0
google-generativeai==0.3.0) to match the comment, or update the comment to
something like "Specify minimum dependency versions" to reflect the current >=
constraints—modify the pip install line or the comment accordingly in the
"Install dependencies" step.
🧹 Nitpick comments (4)
.github/scripts/ci_failure_bot.py (4)
125-136: Narrow exception type for better error handling.Line 134 catches bare
Exception, which can mask unexpected errors. Consider catching specific GitHub API exceptions.🔎 Proposed fix
workflow_file = self.repo.get_contents(workflow_path) return workflow_file.decoded_content.decode("utf-8") - except Exception as e: + except (GithubException, ValueError) as e: print(f"Error getting workflow YAML: {e}") return None
154-154: Remove unusednoqadirective.Line 154 has an unused
noqadirective that can be removed.🔎 Proposed fix
- # noqa: E501 (ignore line length for multiline string) context = f"""
190-195: Narrow exception type in Gemini API call.Line 193 catches bare
Exception. Consider catching specific API exceptions from thegoogle.generativeailibrary for better error diagnostics.🔎 Proposed fix
try: response = self.model.generate_content(context) return response.text - except Exception as e: + except (ValueError, ConnectionError) as e: print(f"Error calling Gemini API: {e}") return self.fallback_response()
246-253: Narrow exception type in actor check.Line 252 catches bare
Exception. Consider catching specific GitHub API exceptions.🔎 Proposed fix
try: workflow_run = self.repo.get_workflow_run(self.workflow_run_id) actor_login = workflow_run.actor.login.lower() if "dependabot" in actor_login: print(f"Skipping dependabot PR from {workflow_run.actor.login}") return - except Exception as e: + except (GithubException, AttributeError) as e: print(f"Warning: Could not check actor: {e}")
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/scripts/ci_failure_bot.py.github/workflows/ci-failure-bot.yml
🧰 Additional context used
🪛 Ruff (0.14.10)
.github/scripts/ci_failure_bot.py
71-71: Probable use of requests call without timeout
(S113)
93-93: Consider moving this statement to an else block
(TRY300)
113-113: Probable use of requests call without timeout
(S113)
120-120: Do not catch blind exception: Exception
(BLE001)
134-134: Do not catch blind exception: Exception
(BLE001)
154-154: Unused noqa directive (non-enabled: E501)
Remove unused noqa directive
(RUF100)
192-192: Consider moving this statement to an else block
(TRY300)
193-193: Do not catch blind exception: Exception
(BLE001)
252-252: Do not catch blind exception: Exception
(BLE001)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
- GitHub Check: Python==3.12 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=5.1.0
- GitHub Check: Python==3.12 | django~=4.2.0
- GitHub Check: Python==3.11 | django~=5.2.0
- GitHub Check: Python==3.11 | django~=5.0.0
- GitHub Check: Python==3.10 | django~=5.0.0
- GitHub Check: Python==3.12 | django~=5.0.0
- GitHub Check: Python==3.13 | django~=5.1.0
- GitHub Check: Python==3.10 | django~=5.2.0
- GitHub Check: Python==3.11 | django~=4.2.0
- GitHub Check: Python==3.10 | django~=5.1.0
- GitHub Check: Python==3.10 | django~=4.2.0
- GitHub Check: Python==3.13 | django~=5.2.0
- GitHub Check: Python==3.11 | django~=5.1.0
🔇 Additional comments (3)
.github/scripts/ci_failure_bot.py (1)
212-239: Comment deduplication logic looks good.The implementation correctly checks for existing bot comments and updates them instead of creating duplicates. The exception handling is appropriately narrow.
.github/workflows/ci-failure-bot.yml (2)
1-18: Workflow trigger and permissions configured correctly.The workflow appropriately:
- Triggers only on CI build failures
- Excludes dependabot PRs at the job level
- Grants minimal required permissions (issues/PR write, contents read)
34-41: Verify GEMINI_API_KEY secret is configured.Line 37 references
secrets.GEMINI_API_KEY. Based on the PR comments, this requires organization-level secret configuration. Ensure the secret is added to the OpenWISP organization before merging, or the workflow will fail.Would you like me to generate a verification script to check if the secret is accessible, or would you prefer to confirm this manually with the maintainers?
2e7b8f1 to
6ab0337
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.github/scripts/ci_failure_bot.py (2)
188-188: Remove unusednoqadirective.The
# noqa: E501directive is unnecessary as E501 (line length) is not enabled in the linter configuration.🔎 Proposed fix
-""" # noqa: E501 +"""
190-195: Consider narrowing the exception type for Gemini API errors.Catching bare
Exceptioncan mask unexpected errors. Thegoogle.generativeailibrary typically raisesgoogle.api_core.exceptions.GoogleAPIErroror similar for API failures. However, given the fallback behavior is appropriate and the bot should be resilient, this is a minor concern.🔎 Proposed fix to catch more specific exceptions
try: response = self.model.generate_content(context) return response.text - except Exception as e: + except (genai.types.GenerateContentError, ValueError, ConnectionError) as e: print(f"Error calling Gemini API: {e}") return self.fallback_response()Note: You may need to verify the exact exception types from the
google-generativeailibrary.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/scripts/ci_failure_bot.py.github/workflows/ci-failure-bot.yml
🧰 Additional context used
🪛 Ruff (0.14.10)
.github/scripts/ci_failure_bot.py
94-94: Consider moving this statement to an else block
(TRY300)
188-188: Unused noqa directive (non-enabled: E501)
Remove unused noqa directive
(RUF100)
192-192: Consider moving this statement to an else block
(TRY300)
193-193: Do not catch blind exception: Exception
(BLE001)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
- GitHub Check: Python==3.10 | django~=5.1.0
- GitHub Check: Python==3.12 | django~=4.2.0
- GitHub Check: Python==3.10 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=5.1.0
- GitHub Check: Python==3.10 | django~=5.0.0
- GitHub Check: Python==3.13 | django~=5.1.0
- GitHub Check: Python==3.12 | django~=5.0.0
- GitHub Check: Python==3.11 | django~=5.0.0
- GitHub Check: Python==3.13 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=5.2.0
- GitHub Check: Python==3.11 | django~=5.1.0
- GitHub Check: Python==3.11 | django~=4.2.0
- GitHub Check: Python==3.11 | django~=5.2.0
- GitHub Check: Python==3.10 | django~=4.2.0
🔇 Additional comments (10)
.github/scripts/ci_failure_bot.py (9)
1-12: LGTM!Imports are well-organized with standard library imports first, followed by third-party packages.
15-54: LGTM!The initialization logic is well-structured with comprehensive environment variable validation, proper integer conversion with error handling, and a configurable Gemini model name via environment variable.
56-97: LGTM!The method correctly fetches failed job logs with a timeout, truncates to avoid token limits, and handles exceptions with specific types.
99-124: LGTM!Previous review feedback was addressed: timeout is set, and exception handling uses specific types.
126-137: LGTM!Simple and focused method with appropriate exception handling.
197-210: LGTM!The fallback response provides useful guidance when AI analysis is unavailable.
212-239: LGTM!The deduplication logic correctly checks for existing bot comments and updates them instead of creating duplicates. This addresses the previous review feedback.
241-271: LGTM!The orchestration flow is clear and includes a defense-in-depth dependabot check. Error handling at each step ensures graceful degradation.
274-276: LGTM!Standard entry point pattern.
.github/workflows/ci-failure-bot.yml (1)
22-27: The action versions are valid. Bothactions/checkout@v6andactions/setup-python@v6are publicly available releases (checkout v6.0.1 and setup-python v6.1.0). The workflow will not fail due to these action versions.Likely an incorrect or invalid review comment.
6ab0337 to
ab7cd1d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI Agents
In @.github/scripts/ci_failure_bot.py:
- Around line 212-239: The comment deduplication in post_comment is brittle
because it looks for specific phrases; update post_comment to use a consistent
hidden HTML marker (e.g., "<!-- ci-failure-bot -->") when creating or editing
comments and check for that marker in existing_comments instead of matching "CI
Build Failed" or "build failed"; ensure the marker is appended to the message
passed to pr.create_issue_comment and comment.edit, and then change the loop
that scans pr.get_issue_comments() to detect comment.body containing the marker
(while preserving the existing bot_login check).
- Around line 99-124: The PR diff extraction in get_pr_diff currently slices
response.text[-8000:], dropping the beginning of the diff where imports and
initial definitions live; change this to capture the start of the diff (e.g.,
use the first 8000 characters of response.text) or, if you want both head and
tail context, concatenate response.text[:4000] + "\n...\n" +
response.text[-4000:] so the function returns the leading context instead of
only the tail.
🧹 Nitpick comments (1)
.github/scripts/ci_failure_bot.py (1)
139-195: Gemini integration is functional but could be refined.The context construction is comprehensive and the system prompt matches the maintainer's requirements. Two observations:
Optional refactor: Line 193 catches bare
Exception, which static analysis flags. While this provides robustness for external API calls, consider catching specificgenaiexceptions if available for clearer error handling.Context size awareness: The combined context (build logs, PR diff, workflow YAML, prompt template) could grow quite large with multiple failed jobs. Gemini 2.5-flash has a generous context window, so this should work, but monitor for potential token limit issues in practice.
🔎 Optional: Narrow exception handling
try: response = self.model.generate_content(context) return response.text - except (ValueError, ConnectionError, Exception) as e: + except (ValueError, ConnectionError, TimeoutError) as e: print(f"Error calling Gemini API: {e}") return self.fallback_response()Note: You may want to check the
google.generativeaidocumentation for specific exception types to catch instead of the bareException.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/scripts/ci_failure_bot.py.github/workflows/ci-failure-bot.yml
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/ci-failure-bot.yml
🧰 Additional context used
🪛 Ruff (0.14.10)
.github/scripts/ci_failure_bot.py
94-94: Consider moving this statement to an else block
(TRY300)
192-192: Consider moving this statement to an else block
(TRY300)
193-193: Do not catch blind exception: Exception
(BLE001)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
- GitHub Check: Python==3.10 | django~=5.0.0
- GitHub Check: Python==3.12 | django~=5.1.0
- GitHub Check: Python==3.11 | django~=4.2.0
- GitHub Check: Python==3.10 | django~=4.2.0
- GitHub Check: Python==3.11 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=5.0.0
- GitHub Check: Python==3.13 | django~=5.1.0
- GitHub Check: Python==3.12 | django~=4.2.0
- GitHub Check: Python==3.12 | django~=5.2.0
- GitHub Check: Python==3.13 | django~=5.2.0
- GitHub Check: Python==3.10 | django~=5.2.0
- GitHub Check: Python==3.11 | django~=5.0.0
- GitHub Check: Python==3.10 | django~=5.1.0
- GitHub Check: Python==3.11 | django~=5.1.0
🔇 Additional comments (3)
.github/scripts/ci_failure_bot.py (3)
16-54: Initialization and environment validation is solid.The constructor properly validates all required environment variables with specific error messages, ensures
workflow_run_idis numeric, and makes the Gemini model configurable. The setup logic is clean and maintainable.
56-97: Build log collection is well-implemented.The method correctly fetches logs from failed jobs with appropriate timeout, limits log size to avoid token overflow, and includes both job-level and step-level failure details. Exception handling is appropriate.
241-271: Main execution flow is well-structured.The
runmethod orchestrates the bot's operations logically: validates actor (skips dependabot), gathers context, analyzes with Gemini, and posts results. The early exit when no build logs are found prevents unnecessary API calls. Good defensive programming throughout.
8e5d108 to
5d3181a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI Agents
In @.github/scripts/ci_failure_bot.py:
- Line 189: Remove the unnecessary inline noqa directive from the trailing
docstring terminator (the '""" # noqa: E501' token) so the line ends simply
with '"""'; update the closing triple-quote in the module-level string in
ci_failure_bot.py to remove the "# noqa: E501" comment.
🧹 Nitpick comments (2)
.github/scripts/ci_failure_bot.py (2)
56-97: Log truncation approach is reasonable for error context.The method correctly fetches logs from failed jobs and steps. Taking the last 5000 characters of logs (line 78) is a pragmatic choice since error messages typically appear at the end of build logs. However, if early setup or environment context is needed for debugging, consider capturing both the first and last portions (e.g., first 2000 + last 3000 chars).
191-196: Consider narrowing the exception catch for better debugging.While catching
Exceptionensures the bot always falls back gracefully, it can mask unexpected errors (e.g., programming bugs). Consider catching only API-related exceptions likegoogle.generativeai.types.GenerationExceptionor logging unexpected exceptions before falling back.🔎 Optional improvement
try: response = self.model.generate_content(context) return response.text - except (ValueError, ConnectionError, Exception) as e: + except (ValueError, ConnectionError, AttributeError) as e: print(f"Error calling Gemini API: {e}") return self.fallback_response() + except Exception as e: + print(f"Unexpected error in Gemini analysis: {e}") + return self.fallback_response()
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/scripts/ci_failure_bot.py.github/workflows/ci-failure-bot.yml
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/ci-failure-bot.yml
🧰 Additional context used
🪛 Ruff (0.14.10)
.github/scripts/ci_failure_bot.py
94-94: Consider moving this statement to an else block
(TRY300)
189-189: Unused noqa directive (non-enabled: E501)
Remove unused noqa directive
(RUF100)
193-193: Consider moving this statement to an else block
(TRY300)
194-194: Do not catch blind exception: Exception
(BLE001)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
- GitHub Check: Python==3.11 | django~=5.0.0
- GitHub Check: Python==3.11 | django~=5.1.0
- GitHub Check: Python==3.11 | django~=5.2.0
- GitHub Check: Python==3.10 | django~=5.2.0
- GitHub Check: Python==3.10 | django~=4.2.0
- GitHub Check: Python==3.13 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=4.2.0
- GitHub Check: Python==3.12 | django~=5.0.0
- GitHub Check: Python==3.12 | django~=5.2.0
- GitHub Check: Python==3.13 | django~=5.1.0
- GitHub Check: Python==3.12 | django~=5.1.0
- GitHub Check: Python==3.10 | django~=5.0.0
- GitHub Check: Python==3.11 | django~=4.2.0
- GitHub Check: Python==3.10 | django~=5.1.0
🔇 Additional comments (7)
.github/scripts/ci_failure_bot.py (7)
1-14: LGTM! Clean imports and documentation.The module docstring and imports are well-organized and appropriate for the bot's functionality.
16-54: LGTM! Robust initialization with comprehensive validation.The environment variable validation is thorough with clear error messages,
workflow_run_idis properly validated as numeric, and the Gemini model is configurable. The initialization sequence is correct.
126-137: LGTM! Clean workflow YAML retrieval.The method correctly fetches and decodes the workflow file content with appropriate error handling.
198-211: LGTM! Appropriate fallback message.The static fallback provides useful guidance when AI analysis is unavailable.
242-277: LGTM! Well-structured main execution flow.The run method orchestrates the bot's workflow logically: validates it's not a dependabot PR, gathers context, performs AI analysis, and posts results. The dependabot check prevents unnecessary processing.
119-119: Diff truncation still loses critical early context.This issue was previously flagged: taking the last 8000 characters discards the beginning of the diff where imports, class definitions, and initial functions typically appear. For large diffs, Gemini will miss crucial context.
Likely an incorrect or invalid review comment.
227-230: Comment deduplication remains brittle.This issue was previously flagged: the string matching for "CI Build Failed" or "build failed" is fragile. If Gemini generates a response without these phrases, duplicate comments will be created. A consistent HTML marker (e.g.,
<!-- ci-failure-bot -->) would be more reliable.Likely an incorrect or invalid review comment.
… bot openwisp#524 Implements complete test coverage for the Gemini AI-powered CI failure bot: - Unit tests for all bot functionality with external API mocking - Comprehensive documentation following OpenWISP patterns - User guide with setup instructions and configuration options - Developer guide with technical implementation details Tests cover initialization, build log retrieval, PR diff handling, Gemini API integration, comment posting, and full workflow execution. Addresses reviewer feedback on automated testing requirements. Closes openwisp#524
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @.github/workflows/ci-failure-bot.yml:
- Around line 29-32: Update the pip install command so it uses a non-vulnerable
requests and the replacement GenAI package: change "requests>=2.31.0" to
"requests>=2.32.4" (or "requests>=2.32.5") and replace
"google-generativeai>=0.3.0" with "google-genai>=0.2.0" in the Install
dependencies run block; then run your dependency lock/update workflow (e.g.,
pip-compile, pip freeze, or CI) to regenerate lock files and verify tests/builds
pass.
In @docs/developer/ci-failure-bot.rst:
- Line 63: The PR_NUMBER assignment using
github.event.workflow_run.pull_requests[0].number can be empty for forked-PR
workflow_run events; either document that limitation or make the step robust by
guarding the job/step with an if condition (e.g., if:
github.event.workflow_run.event == 'pull_request' &&
github.event.workflow_run.pull_requests &&
github.event.workflow_run.pull_requests[0]) so PR_NUMBER isn't set to an empty
string, or implement a fallback that queries the GitHub API for the pull request
number when the pull_requests array is absent.
In @docs/user/ci-failure-bot.rst:
- Around line 106-113: Add explicit limitations stating that PR diffs and build
logs are transmitted to Google's Gemini AI service and describe three impacts:
privacy (sensitive PR content is sent to a third party), compliance
(organizations should review Google/Gemini data retention and handling policies
before enabling), and cost (Gemini API usage fees scale with PR volume and log
size); update the "Limitations" bulleted list (near the existing "Skips
dependabot PRs" and "Requires Google Gemini API access" items) to include these
three new bullets so readers see privacy, compliance, and cost implications
upfront.
🧹 Nitpick comments (1)
.github/scripts/ci_failure_bot.py (1)
148-206: Consider rate limiting and cost management for Gemini API calls.The current implementation makes a Gemini API call for every CI failure without rate limiting or retry logic. This could lead to:
- Cost spikes: Multiple simultaneous CI failures trigger multiple API calls, potentially incurring unexpected costs
- Rate limit exhaustion: High-volume repositories could hit Gemini API rate limits
- Transient failure handling: Network blips cause immediate fallback instead of retry
Consider adding:
- Rate limiting logic to prevent cost spikes (e.g., max N calls per hour)
- Retry with exponential backoff for transient Gemini API failures
- Cost monitoring alerts for Gemini API usage
Would you like me to generate an implementation for rate limiting and retry logic?
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.github/scripts/ci_failure_bot.py.github/workflows/ci-failure-bot.ymldocs/developer/ci-failure-bot.rstdocs/developer/index.rstdocs/index.rstdocs/user/ci-failure-bot.rstopenwisp_utils/tests/test_ci_failure_bot.py
🧰 Additional context used
🧬 Code graph analysis (1)
openwisp_utils/tests/test_ci_failure_bot.py (1)
.github/scripts/ci_failure_bot.py (7)
CIFailureBot(15-294)get_build_logs(56-97)get_pr_diff(99-133)get_workflow_yaml(135-146)analyze_with_gemini(148-205)post_comment(222-250)run(252-294)
🪛 Ruff (0.14.10)
.github/scripts/ci_failure_bot.py
94-94: Consider moving this statement to an else block
(TRY300)
202-202: Consider moving this statement to an else block
(TRY300)
203-203: Do not catch blind exception: Exception
(BLE001)
288-288: Do not catch blind exception: Exception
(BLE001)
301-301: Do not catch blind exception: Exception
(BLE001)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
- GitHub Check: Python==3.11 | django~=5.0.0
- GitHub Check: Python==3.11 | django~=5.2.0
- GitHub Check: Python==3.10 | django~=4.2.0
- GitHub Check: Python==3.13 | django~=5.2.0
- GitHub Check: Python==3.11 | django~=4.2.0
- GitHub Check: Python==3.11 | django~=5.1.0
- GitHub Check: Python==3.12 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=5.1.0
- GitHub Check: Python==3.10 | django~=5.2.0
- GitHub Check: Python==3.13 | django~=5.1.0
- GitHub Check: Python==3.12 | django~=5.0.0
- GitHub Check: Python==3.12 | django~=4.2.0
- GitHub Check: Python==3.10 | django~=5.0.0
- GitHub Check: Python==3.10 | django~=5.1.0
🔇 Additional comments (8)
docs/developer/index.rst (1)
21-21: LGTM!Navigation entry correctly added to the toctree in consistent style with existing documentation structure.
docs/index.rst (1)
40-40: LGTM!Navigation entry correctly added under the user documentation section with consistent formatting.
docs/developer/ci-failure-bot.rst (5)
53-56: Verify dependency versions against actual implementation.The documentation specifies minimum versions for
requests,PyGithub, andgoogle-generativeaipackages. Ensure the actual Python script (ci_failure_bot.py) is compatible with these specified versions and that they align with any other project dependencies.Verify that the actual implementation script uses APIs compatible with these minimum versions:
requests>=2.31.0PyGithub>=2.0.0google-generativeai>=0.3.0
101-108: LGTM!The Notes section clearly communicates important operational constraints: API key configuration, fallback behavior, error handling, required permissions, and Dependabot exclusion. This sets proper expectations for users deploying the workflow.
123-154: LGTM with verification pending.The testing section provides comprehensive guidance with clear test categories (initialization, build logs, PR diffs, Gemini integration, commenting, workflow execution) and mocking strategy. The provided bash commands are practical and actionable. Verify that the actual test file (
openwisp_utils/tests/test_ci_failure_bot.py) implements all claimed test coverage.
180-202: LGTM!The deployment and customization sections are well-structured with clear, actionable steps for adding the workflow to new repositories and customizing behavior. Prompt and trigger customization guidance is appropriate for users who need to adapt the bot to their specific workflows.
213-236: LGTM!The troubleshooting and debugging sections address common development issues (import errors, API rate limits, token limits) and provide concrete debugging commands using
gh run view. This supports developers in understanding and resolving issues independently..github/scripts/ci_failure_bot.py (1)
52-54: No action needed—"gemini-2.5-flash"is the correct and valid identifier for Gemini 2.5 Flash per Google's official API documentation.
5fa94dc to
a372254
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @.github/scripts/ci_failure_bot.py:
- Around line 165-198: The bot's prompt in the context f-string is too harsh and
risks discouraging contributors; update the ROLE, TONE GUIDELINES, and RESPONSE
STRUCTURE text inside the context string (the multiline f-string assigned to
context) to use constructive, neutral phrasing—replace words/phrases like "blunt
assessment", "you neglected", "spam", "drain on project resources", and
"no-nonsense" with alternatives such as "clear assessment", "the test suite
requires updates", "does not meet contribution guidelines" or "potentially
low-quality", and "direct and constructive"; keep the action-oriented
requirement for exact fixes but add guidance to reserve severe language for
clear abuse; ensure the EXAMPLE RESPONSE STYLE is updated accordingly to mirror
the new tone.
- Around line 154-155: The hardcoded "master" in qa_checks_url and runtests_url
breaks links for repos with other default branches; fetch the repository's
default branch via the PyGithub Repo object's default_branch property (e.g.,
repo.default_branch) and use that value instead of the literal "master" when
formatting repo_url-derived links (update the expressions that build
qa_checks_url and runtests_url to interpolate the resolved default_branch); also
handle missing/default_branch gracefully (fallback to "master" or raise a clear
error) to avoid runtime failures.
In @.github/workflows/ci-failure-bot.yml:
- Around line 29-32: Update the pip install line in the "Install dependencies"
step so the minimum package versions are bumped: keep requests>=2.32.5 but
change PyGithub>=2.0.0 to PyGithub>=2.8.1 and google-genai>=0.2.0 to
google-genai>=1.8.0; edit the run block that contains the pip install command to
reflect these new version specifiers so the CI uses the more recent minimum
releases.
🧹 Nitpick comments (5)
.github/scripts/ci_failure_bot.py (4)
49-54: Consider adding error handling for client initialization.The GitHub and Gemini client initialization could fail with invalid credentials or network issues. Consider wrapping these in try-except blocks to provide clearer error messages upfront rather than failing later during API calls.
♻️ Proposed enhancement
+ try: self.github = Github(self.github_token) self.repo = self.github.get_repo(self.repository_name) + except GithubException as e: + print(f"Failed to initialize GitHub client: {e}") + sys.exit(1) + try: genai.configure(api_key=self.gemini_api_key) model_name = os.environ.get("GEMINI_MODEL", "gemini-2.5-flash") self.model = genai.GenerativeModel(model_name) + except Exception as e: + print(f"Failed to initialize Gemini client: {e}") + sys.exit(1)
135-147: Consider truncating workflow YAML content.Unlike
get_build_logs(5000 chars) andget_pr_diff(8000 chars), the workflow YAML content is not truncated. While workflow files are typically small, complex workflows could be large enough to contribute to token limit issues when combined with other context.♻️ Proposed enhancement
def get_workflow_yaml(self): """Get the workflow YAML configuration""" try: workflow_run = self.repo.get_workflow_run(self.workflow_run_id) workflow_path = workflow_run.path # Get workflow file content workflow_file = self.repo.get_contents(workflow_path) - return workflow_file.decoded_content.decode("utf-8") + content = workflow_file.decoded_content.decode("utf-8") + # Truncate if excessively large + if len(content) > 5000: + content = content[:5000] + "\n\n[...truncated...]" + return content except GithubException as e: print(f"Error getting workflow YAML: {e}") return None
200-205: Broad exception catching makes debugging harder.Line 203 catches
Exception, which is very broad and can hide unexpected errors. The static analysis tool correctly flagged this (BLE001). Consider catching more specific exceptions or at least logging the exception type for debugging.♻️ Proposed improvement
try: response = self.model.generate_content(context) return response.text - except (ValueError, ConnectionError, Exception) as e: - print(f"Error calling Gemini API: {e}") + except (ValueError, ConnectionError) as e: + print(f"Error calling Gemini API ({type(e).__name__}): {e}") + return self.fallback_response() + except Exception as e: + print(f"Unexpected error calling Gemini API ({type(e).__name__}): {e}") + import traceback + traceback.print_exc() return self.fallback_response()Based on static analysis hints (Ruff BLE001).
288-294: Broad exception catching (similar issue).Line 288 catches
Exception, which is very broad. While the error handling here includes traceback printing which helps debugging, consider whether all exceptions should result in exit(1) or if some should be handled differently.Based on static analysis hints (Ruff BLE001). This is less critical than line 203 since this is the top-level error handler and includes comprehensive error reporting with traceback.
.github/workflows/ci-failure-bot.yml (1)
42-50: Exit code checking is redundant but provides better logging.The
set -eon line 42 already ensures the script exits immediately on failure, making the explicit exit code check on lines 45-50 technically redundant. However, this pattern provides more informative error messages in the workflow logs, which is valuable for debugging.If you want to simplify while keeping good error messages:
♻️ Alternative approach
run: | set -e # Exit immediately if any command fails echo "Starting CI Failure Bot..." python .github/scripts/ci_failure_bot.py - exit_code=$? - if [ $exit_code -ne 0 ]; then - echo "ERROR: CI Failure Bot failed with exit code $exit_code" - echo "This indicates a critical issue with the bot logic that needs immediate attention" - exit $exit_code - fi echo "CI Failure Bot completed successfully"However, the current approach is acceptable and provides clearer error context.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/scripts/ci_failure_bot.py.github/workflows/ci-failure-bot.ymldocs/developer/ci-failure-bot.rstdocs/user/ci-failure-bot.rst
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/user/ci-failure-bot.rst
🧰 Additional context used
🪛 Ruff (0.14.10)
.github/scripts/ci_failure_bot.py
94-94: Consider moving this statement to an else block
(TRY300)
202-202: Consider moving this statement to an else block
(TRY300)
203-203: Do not catch blind exception: Exception
(BLE001)
288-288: Do not catch blind exception: Exception
(BLE001)
301-301: Do not catch blind exception: Exception
(BLE001)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
- GitHub Check: Python==3.13 | django~=5.2.0
- GitHub Check: Python==3.13 | django~=5.1.0
- GitHub Check: Python==3.12 | django~=5.1.0
- GitHub Check: Python==3.11 | django~=5.0.0
- GitHub Check: Python==3.10 | django~=5.0.0
- GitHub Check: Python==3.11 | django~=5.1.0
- GitHub Check: Python==3.10 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=4.2.0
- GitHub Check: Python==3.10 | django~=4.2.0
- GitHub Check: Python==3.11 | django~=4.2.0
- GitHub Check: Python==3.10 | django~=5.1.0
- GitHub Check: Python==3.11 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=5.0.0
🔇 Additional comments (2)
.github/scripts/ci_failure_bot.py (1)
222-251: Excellent comment deduplication implementation.The deduplication logic using a marker and checking for existing comments before creating new ones is well-implemented. This prevents comment spam on PRs and provides a better user experience by updating a single comment with the latest analysis.
.github/workflows/ci-failure-bot.yml (1)
10-13: LGTM: Minimal required permissions.The workflow follows the principle of least privilege by requesting only the necessary permissions: write access for comments and read access for repository contents.
| qa_checks_url = f"{repo_url}/blob/master/openwisp-qa-check" | ||
| runtests_url = f"{repo_url}/blob/master/runtests" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# First, locate and read the file around the mentioned lines
cd /tmp/repo 2>/dev/null || true
cat -n .github/scripts/ci_failure_bot.py | sed -n '140,170p'Repository: openwisp/openwisp-utils
Length of output: 1616
🏁 Script executed:
# Check if this is a GitHub API related class to understand what self.repo is
cat -n .github/scripts/ci_failure_bot.py | head -50Repository: openwisp/openwisp-utils
Length of output: 1975
🏁 Script executed:
# Check the repository's actual default branch
git symbolic-ref refs/remotes/origin/HEAD | sed 's@.*origin/@@'Repository: openwisp/openwisp-utils
Length of output: 73
🏁 Script executed:
# Check PyGithub version and imports
grep -r "github\|PyGithub" .github/scripts/ci_failure_bot.py setup.py setup.cfg requirements.txt 2>/dev/null | head -20Repository: openwisp/openwisp-utils
Length of output: 1228
🏁 Script executed:
# Search for any existing usage of default_branch in the codebase
rg "default_branch" --type pythonRepository: openwisp/openwisp-utils
Length of output: 97
🏁 Script executed:
# Check PyGithub documentation for Repository class attributes
python3 << 'EOF'
try:
from github import Repository
repo_class = Repository.Repository
# Check if default_branch is available
if hasattr(repo_class, 'default_branch'):
print("default_branch attribute exists in Repository class")
# Try to inspect the attributes
import inspect
members = inspect.getmembers(repo_class)
default_branch_members = [m for m in members if 'default' in m[0].lower() or 'branch' in m[0].lower()]
print(f"Found {len(default_branch_members)} relevant attributes:")
for name, _ in default_branch_members[:10]:
print(f" - {name}")
except Exception as e:
print(f"Error: {e}")
EOFRepository: openwisp/openwisp-utils
Length of output: 98
🏁 Script executed:
# Check if there's a requirements file to see PyGithub version
find . -name "requirements*.txt" -o -name "setup.py" -o -name "setup.cfg" -o -name "pyproject.toml" | head -5 | xargs cat 2>/dev/null | grep -i "github\|pygithub"Repository: openwisp/openwisp-utils
Length of output: 223
🏁 Script executed:
# Let me check the actual PyGithub documentation by looking at common usage patterns
web_search "PyGithub Repository default_branch attribute"Repository: openwisp/openwisp-utils
Length of output: 117
🏁 Script executed:
# Also check if there are any other references to branch names in the codebase
rg "blob/.*/" .github/scripts/ --type python -B 2 -A 2Repository: openwisp/openwisp-utils
Length of output: 97
🏁 Script executed:
# Check the setup.py to see PyGithub version specification
cat setup.py | grep -A 20 -B 5 "install_requires\|dependencies"Repository: openwisp/openwisp-utils
Length of output: 963
🌐 Web query:
PyGithub Repository class default_branch property documentation
💡 Result:
The Repository.default_branch property is a read-only string property that returns the repository’s default branch name (e.g., "main"). To change it use repo.edit(default_branch='branch-name') rather than assigning to the attribute. [1][2]
Example:
from github import Github
repo = Github().get_repo('owner/repo')
print(repo.default_branch) # returns string
repo.edit(default_branch='release') # to change the default branch
Sources:
[1] PyGithub Repository docs — property default_branch.
[2] StackOverflow: default_branch is read-only; use repo.edit(...).
🏁 Script executed:
# Let me check if there are other files that might show PyGithub version requirements
fd -e txt -e cfg -e toml -e yaml | xargs grep -l "PyGithub\|pygithub" 2>/dev/nullRepository: openwisp/openwisp-utils
Length of output: 51
🏁 Script executed:
# Check all Python files in .github/scripts to see complete context
ls -la .github/scripts/*.pyRepository: openwisp/openwisp-utils
Length of output: 150
Replace hardcoded "master" branch with dynamic default branch resolution.
Lines 154-155 hardcode the "master" branch name in URLs. Repositories using "main" or other default branch names will have broken links. Use PyGithub's default_branch property to dynamically resolve the branch name.
🔧 Proposed fix
repo_url = f"https://github.com/{self.repository_name}"
- qa_checks_url = f"{repo_url}/blob/master/openwisp-qa-check"
- runtests_url = f"{repo_url}/blob/master/runtests"
+ default_branch = self.repo.default_branch
+ qa_checks_url = f"{repo_url}/blob/{default_branch}/openwisp-qa-check"
+ runtests_url = f"{repo_url}/blob/{default_branch}/runtests"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| qa_checks_url = f"{repo_url}/blob/master/openwisp-qa-check" | |
| runtests_url = f"{repo_url}/blob/master/runtests" | |
| repo_url = f"https://github.com/{self.repository_name}" | |
| default_branch = self.repo.default_branch | |
| qa_checks_url = f"{repo_url}/blob/{default_branch}/openwisp-qa-check" | |
| runtests_url = f"{repo_url}/blob/{default_branch}/runtests" |
🤖 Prompt for AI Agents
In @.github/scripts/ci_failure_bot.py around lines 154 - 155, The hardcoded
"master" in qa_checks_url and runtests_url breaks links for repos with other
default branches; fetch the repository's default branch via the PyGithub Repo
object's default_branch property (e.g., repo.default_branch) and use that value
instead of the literal "master" when formatting repo_url-derived links (update
the expressions that build qa_checks_url and runtests_url to interpolate the
resolved default_branch); also handle missing/default_branch gracefully
(fallback to "master" or raise a clear error) to avoid runtime failures.
| - name: Install dependencies | ||
| # Install minimum required versions | ||
| run: | | ||
| pip install requests>=2.32.5 PyGithub>=2.0.0 google-genai>=0.2.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🌐 Web query:
latest version requests python pip package 2025
💡 Result:
As of January 9, 2026 the latest released version of the requests PyPI package is 2.32.5 (uploaded Aug 18, 2025). [1][2]
Sources:
- PyPI requests project (release files and upload date). [1]
- Safety DB listing (latest version 2.32.5). [2]
🌐 Web query:
latest version PyGithub python pip package 2025
💡 Result:
The latest PyPI release of PyGithub is 2.8.1 (uploaded Sep 2, 2025). [1][2]
References:
[1] PyPI project page — PyGithub (pygithub 2.8.1).
[2] PyGithub changelog / documentation (version 2.8.1).
🌐 Web query:
latest version google-genai python pip package 2025
💡 Result:
The latest google-genai PyPI release in 2025 was version 1.8.0 (uploaded March 26, 2025). [1]
Sources:
- PyPI release page for google-genai (1.8.0). [1]
🌐 Web query:
security vulnerabilities requests 2.32.5 PyGithub 2.0.0 google-genai 0.2.0
💡 Result:
Summary — current public info (searched Jan 9, 2026):
-
requests 2.32.5 — No known unpatched CVEs in 2.32.5; vulnerabilities like CVE-2024-35195 (Session verify=False behavior) and CVE-2024-47081 were fixed in the 2.32.x line (upgrade to ≥2.32.0/2.32.4+ as advised). [1][2][3]
-
PyGithub 2.0.0 — I found no public CVE/official security advisory specifically affecting PyGithub 2.0.0; major vulnerability databases list no direct CVEs for current PyGithub releases (still recommend using a maintained/latest release). [4][5]
-
google-genai 0.2.0 — I found no public CVEs or security advisories tied to version 0.2.0 in major vulnerability databases (no direct vulnerabilities reported), but the package has had many subsequent releases — consider upgrading to a maintained release. [6][7]
Recommended action: if you run these versions in production, update to the latest stable releases (or at least the minimum patched releases noted above for requests) and rescans your dependencies with your SCA tool of choice.
Sources:
- Rapid7 / CVE-2024-35195 (requests fix notes).
- Snyk / requests package page (no direct vulnerabilities in 2.32.5).
- Safety DB / requests vulnerabilities & release notes.
- Snyk / PyGithub package page (no direct vulnerabilities listed).
- PyGithub changelog / releases.
- Snyk / @google/genai / google-genai entries (no direct vulnerabilities listed).
- PyPI google-genai project page.
Update minimum dependency versions to more recent releases.
While the specified versions are free from known unpatched vulnerabilities, the minimum versions for PyGithub and google-genai are significantly outdated:
requests>=2.32.5is appropriate (latest stable as of January 2026)PyGithub>=2.0.0should bePyGithub>=2.8.1(latest as of September 2025)google-genai>=0.2.0should begoogle-genai>=1.8.0(latest as of March 2025)
Updating these minimums ensures the CI workflow uses current, well-maintained releases.
🤖 Prompt for AI Agents
In @.github/workflows/ci-failure-bot.yml around lines 29 - 32, Update the pip
install line in the "Install dependencies" step so the minimum package versions
are bumped: keep requests>=2.32.5 but change PyGithub>=2.0.0 to PyGithub>=2.8.1
and google-genai>=0.2.0 to google-genai>=1.8.0; edit the run block that contains
the pip install command to reflect these new version specifiers so the CI uses
the more recent minimum releases.
a372254 to
428ec11
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @.github/scripts/ci_failure_bot.py:
- Around line 148-206: The analyze_with_gemini method builds a multi-line
f-string prompt that currently contains inline linting comments like "# noqa:
E501" which will be sent to the AI; remove all occurrences of these "# noqa"
annotations inside the context string (lines referencing the ROLE, Project Name,
run-qa-checks, runtests, TONE GUIDELINES, and RESPONSE STRUCTURE parts) so the
prompt text is clean, keeping the rest of the f-string content and behavior
unchanged; after editing the context string, run the same try/except around
model.generate_content and ensure the fallback_response path (caught by the
existing Exception clause) remains intact.
In @docs/developer/ci-failure-bot.rst:
- Around line 1-67: Update the dependency spec in the workflow's "Install
dependencies" step: replace the nonexistent version constraint
`google-genai>=0.2.0` with `google-genai>=1.0.0` so the runtime matches the SDK
that provides `configure()` and `GenerativeModel()` used by the script; ensure
the pip install command line (the step that runs `pip install requests>=2.32.5
PyGithub>=2.0.0 google-genai>=0.2.0`) is edited accordingly.
🧹 Nitpick comments (1)
.github/scripts/ci_failure_bot.py (1)
56-98: Consider truncating logs with first+last sections.The method correctly retrieves failed job logs, but truncates to only the last 5000 characters. This differs from the PR diff strategy (lines 117-123) which preserves both first and last sections. Early CI errors (imports, setup, environment issues) are often critical for diagnosis and might be missed with tail-only truncation.
♻️ Consider this truncation strategy
response = requests.get(logs_url, headers=headers, timeout=30) if response.status_code == 200: + log_text = response.text + if len(log_text) > 5000: + # Take first 2500 and last 2500 for context + log_text = ( + log_text[:2500] + + "\n\n[...middle truncated...]\n\n" + + log_text[-2500:] + ) build_logs.append( { "job_name": job.name, - "logs": response.text[ - -5000: - ], # Last 5000 chars to avoid token limits + "logs": log_text, } )
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/scripts/ci_failure_bot.py.github/workflows/ci-failure-bot.ymldocs/developer/ci-failure-bot.rstdocs/user/ci-failure-bot.rst
🚧 Files skipped from review as they are similar to previous changes (2)
- .github/workflows/ci-failure-bot.yml
- docs/user/ci-failure-bot.rst
🧰 Additional context used
🪛 Ruff (0.14.10)
.github/scripts/ci_failure_bot.py
94-94: Consider moving this statement to an else block
(TRY300)
202-202: Consider moving this statement to an else block
(TRY300)
203-203: Do not catch blind exception: Exception
(BLE001)
288-288: Do not catch blind exception: Exception
(BLE001)
301-301: Do not catch blind exception: Exception
(BLE001)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
- GitHub Check: Python==3.13 | django~=5.1.0
- GitHub Check: Python==3.12 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=5.1.0
- GitHub Check: Python==3.11 | django~=5.2.0
- GitHub Check: Python==3.11 | django~=5.1.0
- GitHub Check: Python==3.13 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=4.2.0
- GitHub Check: Python==3.10 | django~=5.2.0
- GitHub Check: Python==3.10 | django~=5.1.0
- GitHub Check: Python==3.12 | django~=5.0.0
- GitHub Check: Python==3.10 | django~=5.0.0
- GitHub Check: Python==3.10 | django~=4.2.0
- GitHub Check: Python==3.11 | django~=4.2.0
- GitHub Check: Python==3.11 | django~=5.0.0
🔇 Additional comments (10)
.github/scripts/ci_failure_bot.py (6)
1-14: LGTM: Clean imports and structure.The shebang, docstring, and imports are well-organized and appropriate for the bot's functionality.
99-134: LGTM: Well-implemented PR diff retrieval.The method handles edge cases gracefully (missing PR number) and uses an intelligent truncation strategy that preserves both beginning and end context.
135-147: LGTM: Clean workflow YAML retrieval.The method correctly fetches and decodes the workflow file with appropriate error handling.
207-251: LGTM: Excellent fallback and comment deduplication logic.The fallback response provides useful guidance when AI is unavailable, and the comment deduplication using a marker is well-designed to prevent spam.
252-306: LGTM: Robust main execution flow.The method correctly orchestrates the bot workflow with good error handling. The bare
Exceptioncatches at lines 288 and 301 are appropriate for top-level exception handlers in a standalone script, ensuring failures are logged before exiting.
16-55: Gemini model identifier is valid.The default model
gemini-2.5-flashis the correct identifier for Google Gemini 2.5 Flash in the google-genai Python library.docs/developer/ci-failure-bot.rst (4)
68-100: LGTM: Well-documented features and examples.The features list accurately reflects the bot's capabilities, and the response examples effectively illustrate the expected behavior for different failure scenarios.
101-154: LGTM: Comprehensive implementation and testing documentation.The architecture description, testing strategy, and execution instructions are thorough and accurate, providing clear guidance for developers working with the bot.
155-203: LGTM: Clear configuration and deployment guidance.The environment variables, permissions, and deployment steps are accurately documented and align with the bot's implementation.
204-257: LGTM: Thorough troubleshooting and future planning.The troubleshooting guidance, debugging commands, future enhancement ideas, and contributing guidelines provide excellent support for developers working with or extending the bot.
| def analyze_with_gemini(self, build_logs, pr_diff, workflow_yaml): | ||
| """Send context to Gemini for intelligent analysis""" | ||
|
|
||
| # Prepare context for Gemini | ||
| project_name = self.repository_name.split("/")[-1] | ||
| repo_url = f"https://github.com/{self.repository_name}" | ||
| qa_checks_url = f"{repo_url}/blob/master/openwisp-qa-check" | ||
| runtests_url = f"{repo_url}/blob/master/runtests" | ||
|
|
||
| # Build the context string with proper line breaks | ||
| build_logs_json = json.dumps(build_logs, indent=2) | ||
| if pr_diff: | ||
| pr_diff_json = json.dumps(pr_diff, indent=2) | ||
| else: | ||
| pr_diff_json = "No PR associated" | ||
|
|
||
| # Gemini prompt - ignore line length for readability | ||
| context = f""" | ||
| ### ROLE | ||
| You are the "Automated Maintainer Gatekeeper." Your goal is to analyze Pull Request (PR) build failures and provide direct, technically accurate, and no-nonsense feedback to contributors. # noqa: E501 | ||
|
|
||
| ### INPUT CONTEXT PROVIDED | ||
| 1. **Build Output/Logs:** {build_logs_json} | ||
| 2. **YAML Workflow:** {workflow_yaml or "Not available"} | ||
| 3. **PR Diff:** {pr_diff_json} | ||
| 4. **Project Name:** {project_name} # noqa: E501 | ||
| 5. **Repository:** {repo_url} | ||
| 6. **run-qa-checks:** {qa_checks_url} # noqa: E501 | ||
| 7. **runtests:** {runtests_url} # noqa: E501 | ||
|
|
||
| ### TASK | ||
| Analyze the provided context to determine why the build failed. Categorize the failure and respond according to the "Tone Guidelines" below. # noqa: E501 | ||
|
|
||
| ### TONE GUIDELINES | ||
| - **Direct & Honest:** Do not use "fluff" or overly polite corporate language. | ||
| - **Firm Standards:** If a PR is low-effort, spammy, or fails to follow basic instructions, state that clearly. | ||
| - **Action-Oriented:** Provide the exact command or file change needed to fix the error, unless the PR is spammy, in which case we should just declare the PR as potential SPAM and ask maintainers to manually review it. # noqa: E501 | ||
|
|
||
| ### RESPONSE STRUCTURE | ||
| 1. **Status Summary:** A one-sentence blunt assessment of the failure. | ||
| 2. **Technical Diagnosis:** | ||
| - Identify the specific line/test that failed. | ||
| - Explain *why* it failed. | ||
| 3. **Required Action:** Provide a code block or specific steps the contributor must take. | ||
| 4. **Quality Warning (If Applicable):** If the PR appears to be "spam" (e.g., trivial README changes, AI-generated nonsense, or repeated basic errors), include a firm statement that such contributions are a drain on project resources and ping the maintainers asking them for manual review. # noqa: E501 | ||
|
|
||
| ### EXAMPLE RESPONSE STYLE | ||
| "The build failed because you neglected to update the test suite to match your logic changes. This project does not accept functional changes without corresponding test updates. Refer to the log at line 452. Update tests/logic_test.py before re-submitting. We prioritize high-quality, ready-to-merge code; please ensure you run local tests before pushing." # noqa: E501 | ||
|
|
||
| Analyze the failure and provide your response: | ||
| """ | ||
|
|
||
| try: | ||
| response = self.model.generate_content(context) | ||
| return response.text | ||
| except (ValueError, ConnectionError, Exception) as e: | ||
| print(f"Error calling Gemini API: {e}") | ||
| return self.fallback_response() | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚫 SKIP
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Srinath0916, understood! I'll skip this suggestion. Feel free to proceed as you see fit. 👍
428ec11 to
729b3d8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.github/scripts/ci_failure_bot.py:
- Around line 151-156: The hardcoded branch and file paths in repo URL
construction (variables repo_url, qa_checks_url, runtests_url using
self.repository_name and "master") make links brittle; update the class to
accept configurable branch and path names (e.g., add optional
parameters/defaults for branch, qa_path, runtests_path) or detect the repository
default branch via the GitHub API and use that value when building
repo_url/qa_checks_url/runtests_url, falling back to a sensible default; ensure
updates are applied where project_name, repo_url, qa_checks_url, and
runtests_url are computed so links remain valid across repos and branch name
changes.
🧹 Nitpick comments (1)
.github/scripts/ci_failure_bot.py (1)
56-97: Consider standardizing the build_logs data structure.The
build_logslist contains entries with different schemas:
- Lines 74-81: Entries with
job_nameandlogs- Lines 86-92: Entries with
job_name,step_name, andstep_numberThis inconsistency could complicate downstream processing. Consider either:
- Using separate lists for job logs and step metadata
- Using a consistent schema with optional fields clearly marked
♻️ Proposed standardization
if response.status_code == 200: build_logs.append( { "job_name": job.name, + "type": "job_logs", "logs": response.text[ ], # Last 5000 chars to avoid token limits + "step_name": None, + "step_number": None, } ) # Also get step details for step in job.steps: if step.conclusion == "failure": build_logs.append( { "job_name": job.name, + "type": "step_metadata", "step_name": step.name, "step_number": step.number, + "logs": None, } )
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/scripts/ci_failure_bot.py.github/workflows/ci-failure-bot.ymldocs/developer/ci-failure-bot.rstdocs/user/ci-failure-bot.rst
✅ Files skipped from review due to trivial changes (1)
- docs/user/ci-failure-bot.rst
🚧 Files skipped from review as they are similar to previous changes (2)
- docs/developer/ci-failure-bot.rst
- .github/workflows/ci-failure-bot.yml
🧰 Additional context used
🪛 Ruff (0.14.10)
.github/scripts/ci_failure_bot.py
94-94: Consider moving this statement to an else block
(TRY300)
202-202: Consider moving this statement to an else block
(TRY300)
203-203: Do not catch blind exception: Exception
(BLE001)
288-288: Do not catch blind exception: Exception
(BLE001)
301-301: Do not catch blind exception: Exception
(BLE001)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
- GitHub Check: Python==3.13 | django~=5.1.0
- GitHub Check: Python==3.12 | django~=4.2.0
- GitHub Check: Python==3.10 | django~=4.2.0
- GitHub Check: Python==3.11 | django~=5.1.0
- GitHub Check: Python==3.12 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=5.1.0
- GitHub Check: Python==3.12 | django~=5.0.0
- GitHub Check: Python==3.10 | django~=5.1.0
- GitHub Check: Python==3.10 | django~=5.2.0
- GitHub Check: Python==3.11 | django~=4.2.0
- GitHub Check: Python==3.13 | django~=5.2.0
- GitHub Check: Python==3.11 | django~=5.2.0
- GitHub Check: Python==3.10 | django~=5.0.0
- GitHub Check: Python==3.11 | django~=5.0.0
🔇 Additional comments (8)
.github/scripts/ci_failure_bot.py (8)
1-12: LGTM!The imports are well-organized and appropriate for the bot's functionality.
99-133: LGTM!The method handles edge cases well (missing/empty PR number), uses appropriate timeouts, and implements a smart truncation strategy that preserves context from both ends of large diffs.
135-146: LGTM!Clean implementation with appropriate error handling.
207-220: LGTM!The fallback response provides helpful troubleshooting guidance when AI analysis is unavailable.
222-250: LGTM!The comment deduplication logic is well-implemented, preventing multiple bot comments while allowing updates to existing ones.
252-294: LGTM!The orchestration flow is well-structured with appropriate safety checks. The broad exception handling at line 288 is acceptable for a top-level run method where we want to catch and log any unexpected failures.
Note: Static analysis flags about catching bare
Exceptioncan be safely ignored here, as this is the top-level error handler that should catch all failures for logging.
297-306: LGTM!The main execution block provides appropriate top-level error handling to ensure failures are logged before the script exits.
52-54: No action required. The default model name "gemini-2.5-flash" is a valid and currently available model in the Google Gemini API as of January 2026.
| # Prepare context for Gemini | ||
| project_name = self.repository_name.split("/")[-1] | ||
| repo_url = f"https://github.com/{self.repository_name}" | ||
| qa_checks_url = f"{repo_url}/blob/master/openwisp-qa-check" | ||
| runtests_url = f"{repo_url}/blob/master/runtests" | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check repository structure and default branch
echo "=== Checking default branch ==="
git symbolic-ref refs/remotes/origin/HEAD 2>/dev/null | sed 's@^refs/remotes/origin/@@' || echo "Could not determine default branch from git"
echo -e "\n=== Checking for openwisp-qa-check file ==="
fd -t f 'openwisp-qa-check' 2>/dev/null || echo "Not found with fd"
find . -name 'openwisp-qa-check' -type f 2>/dev/null || echo "Not found with find"
echo -e "\n=== Checking for runtests file ==="
fd -t f 'runtests' 2>/dev/null || echo "Not found with fd"
find . -name 'runtests' -type f 2>/dev/null || echo "Not found with find"
echo -e "\n=== Checking repository root structure ==="
ls -la . | head -20Repository: openwisp/openwisp-utils
Length of output: 1488
🏁 Script executed:
#!/bin/bash
# Examine the ci_failure_bot.py file around the flagged lines
echo "=== Viewing ci_failure_bot.py lines 145-160 ==="
sed -n '145,160p' .github/scripts/ci_failure_bot.py
echo -e "\n=== Viewing ci_failure_bot.py lines 190-210 ==="
sed -n '190,210p' .github/scripts/ci_failure_bot.pyRepository: openwisp/openwisp-utils
Length of output: 2119
🏁 Script executed:
#!/bin/bash
# Check if there are any other references to master branch or these files in the codebase
echo "=== Searching for 'blob/master' references ==="
rg 'blob/master' --type py 2>/dev/null | head -20
echo -e "\n=== Searching for openwisp-qa-check references ==="
rg 'openwisp-qa-check' 2>/dev/null | head -20
echo -e "\n=== Searching for runtests references ==="
rg 'runtests' --type py 2>/dev/null | head -20Repository: openwisp/openwisp-utils
Length of output: 1266
Consider making branch name and repository paths dynamic or configurable.
While openwisp-qa-check and runtests currently exist at the repository root and the project uses master as the default branch, the hardcoded branch name and paths in lines 154-155 create brittleness:
- If the project switches to
main(increasingly common), these URLs become invalid - If file locations or names change, the links become stale without code updates
- The method accepts
self.repository_nameas a parameter, suggesting potential reuse across projects
Consider detecting the default branch dynamically via the GitHub API or making these paths configurable.
🤖 Prompt for AI Agents
In @.github/scripts/ci_failure_bot.py around lines 151 - 156, The hardcoded
branch and file paths in repo URL construction (variables repo_url,
qa_checks_url, runtests_url using self.repository_name and "master") make links
brittle; update the class to accept configurable branch and path names (e.g.,
add optional parameters/defaults for branch, qa_path, runtests_path) or detect
the repository default branch via the GitHub API and use that value when
building repo_url/qa_checks_url/runtests_url, falling back to a sensible
default; ensure updates are applied where project_name, repo_url, qa_checks_url,
and runtests_url are computed so links remain valid across repos and branch name
changes.
729b3d8 to
0bc5dcc
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
.github/scripts/ci_failure_bot.py (2)
152-156: Hardcoded repository-specific URLs may not exist in other repositories.The URLs
openwisp-qa-checkandruntestsare hardcoded assuming they exist at the repository root. If this bot is intended for use across multiple OpenWISP repositories or beyond, these files may not exist, leading to broken links in the AI context.Consider making these paths configurable via environment variables or detecting their existence before including them in the prompt.
291-293: Movetracebackimport to the top of the file.Importing
tracebackinside the exception handler works but is unconventional. Top-level imports are preferred for clarity and consistency.Proposed fix
At the top of the file (after line 8):
import tracebackThen remove the inline imports at lines 291 and 303.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/scripts/ci_failure_bot.py.github/workflows/ci-failure-bot.ymldocs/developer/ci-failure-bot.rstdocs/user/ci-failure-bot.rst
🚧 Files skipped from review as they are similar to previous changes (2)
- docs/developer/ci-failure-bot.rst
- .github/workflows/ci-failure-bot.yml
🧰 Additional context used
🪛 Ruff (0.14.10)
.github/scripts/ci_failure_bot.py
94-94: Consider moving this statement to an else block
(TRY300)
202-202: Consider moving this statement to an else block
(TRY300)
203-203: Do not catch blind exception: Exception
(BLE001)
288-288: Do not catch blind exception: Exception
(BLE001)
301-301: Do not catch blind exception: Exception
(BLE001)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
- GitHub Check: Python==3.12 | django~=5.2.0
- GitHub Check: Python==3.10 | django~=5.2.0
- GitHub Check: Python==3.10 | django~=4.2.0
- GitHub Check: Python==3.12 | django~=5.0.0
- GitHub Check: Python==3.11 | django~=5.2.0
- GitHub Check: Python==3.13 | django~=5.2.0
- GitHub Check: Python==3.11 | django~=5.0.0
- GitHub Check: Python==3.11 | django~=5.1.0
- GitHub Check: Python==3.10 | django~=5.1.0
- GitHub Check: Python==3.13 | django~=5.1.0
- GitHub Check: Python==3.12 | django~=5.1.0
- GitHub Check: Python==3.11 | django~=4.2.0
- GitHub Check: Python==3.10 | django~=5.0.0
- GitHub Check: Python==3.12 | django~=4.2.0
🔇 Additional comments (8)
docs/user/ci-failure-bot.rst (1)
1-89: Documentation is well-structured and comprehensive.The documentation covers all essential aspects: features, workflow, configuration, troubleshooting, and limitations. The privacy and API cost considerations (lines 77-84) are particularly important disclosures for organizations evaluating this feature.
.github/scripts/ci_failure_bot.py (7)
16-54: Initialization logic is robust.Environment variable validation with specific error messages for missing variables, proper type conversion for
workflow_run_id, and sensible defaults forGEMINI_MODELare all handled correctly.
99-133: LGTM!Proper validation of PR number, sensible diff truncation strategy to avoid token limits, and comprehensive exception handling.
135-146: LGTM!Clean implementation for fetching workflow YAML content.
203-205: Broad exception handling is acceptable here.While static analysis flags the catch-all
Exception, for an external AI API call where failures should not crash the CI bot, falling back to a helpful default response is pragmatic. The error is logged for debugging.
207-220: LGTM!The fallback message provides actionable guidance when AI analysis is unavailable.
222-250: LGTM!Comment deduplication using markers and updating existing comments rather than creating duplicates is a clean approach that prevents notification spam.
297-306: LGTM!The main block properly catches and logs any unexpected failures, ensuring the CI workflow receives appropriate exit codes.
0bc5dcc to
4c4291e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.github/scripts/ci_failure_bot.py:
- Around line 152-156: The code builds qa_checks_url and runtests_url using a
hardcoded "master" branch, causing broken links for repos using "main" or
another default branch; update the logic that constructs qa_checks_url and
runtests_url to use the repository's actual default branch (e.g., fetch and use
a default_branch value from the repo metadata or a GitHub API call) instead of
"master", and reference the same repository_name-derived repo_url; modify the
variables qa_checks_url and runtests_url to interpolate that default branch
variable (keep project_name and repo_url usage intact).
🧹 Nitpick comments (1)
.github/scripts/ci_failure_bot.py (1)
77-79: Consider capturing both early and late log content.Taking only the last 5000 characters might miss critical early errors (e.g., dependency installation failures, environment setup issues). Consider capturing first 2500 + last 2500 characters to provide better diagnostic context.
♻️ Alternative truncation approach
- build_logs.append( - { - "job_name": job.name, - "logs": response.text[ - -5000: - ], # Last 5000 chars to avoid token limits - } - ) + log_text = response.text + if len(log_text) > 5000: + # Capture first 2500 and last 2500 for better context + log_text = ( + log_text[:2500] + + "\n\n[...middle truncated...]\n\n" + + log_text[-2500:] + ) + build_logs.append( + { + "job_name": job.name, + "logs": log_text, + } + )
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/scripts/ci_failure_bot.py.github/workflows/ci-failure-bot.ymldocs/developer/ci-failure-bot.rstdocs/user/ci-failure-bot.rst
🚧 Files skipped from review as they are similar to previous changes (3)
- .github/workflows/ci-failure-bot.yml
- docs/developer/ci-failure-bot.rst
- docs/user/ci-failure-bot.rst
🧰 Additional context used
🪛 Ruff (0.14.10)
.github/scripts/ci_failure_bot.py
94-94: Consider moving this statement to an else block
(TRY300)
202-202: Consider moving this statement to an else block
(TRY300)
203-203: Do not catch blind exception: Exception
(BLE001)
288-288: Do not catch blind exception: Exception
(BLE001)
301-301: Do not catch blind exception: Exception
(BLE001)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
- GitHub Check: Python==3.10 | django~=5.0.0
- GitHub Check: Python==3.13 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=5.2.0
- GitHub Check: Python==3.13 | django~=5.1.0
- GitHub Check: Python==3.10 | django~=4.2.0
- GitHub Check: Python==3.10 | django~=5.1.0
- GitHub Check: Python==3.12 | django~=4.2.0
- GitHub Check: Python==3.11 | django~=4.2.0
- GitHub Check: Python==3.12 | django~=5.0.0
- GitHub Check: Python==3.11 | django~=5.2.0
- GitHub Check: Python==3.10 | django~=5.2.0
- GitHub Check: Python==3.11 | django~=5.0.0
- GitHub Check: Python==3.11 | django~=5.1.0
- GitHub Check: Python==3.12 | django~=5.1.0
🔇 Additional comments (9)
.github/scripts/ci_failure_bot.py (9)
1-14: LGTM!The imports are appropriate and well-organized for the bot's functionality.
16-54: LGTM!The initialization logic is solid with comprehensive environment variable validation and clear error messages. The optional
GEMINI_MODELoverride provides good flexibility.
99-133: LGTM!The PR diff retrieval handles edge cases well (missing PR number, empty strings) and includes appropriate error handling. The truncation strategy preserves context from both the beginning and end of large diffs.
135-146: LGTM!Workflow YAML retrieval is straightforward with appropriate error handling.
200-205: LGTM - Exception handling is appropriate here.The broad exception catch on line 203 (flagged by static analysis) is acceptable in this context because:
- It's a safety net for external API calls with unpredictable failure modes
- Falls back gracefully to
fallback_response()- Logs errors for debugging
207-220: LGTM!The fallback response provides useful generic guidance when AI analysis is unavailable.
236-244: LGTM - Deduplication logic is sound.The approach of iterating through comments to find existing bot comments is necessary for proper deduplication. While this could be slow on PRs with hundreds of comments, such cases are rare and the API doesn't provide better filtering options.
252-294: LGTM - Main execution flow is well-structured.The broad exception catch on line 288 (flagged by static analysis) is appropriate as a top-level error handler. It ensures failures are logged with full tracebacks and the script exits with proper error codes.
The dependabot check provides defense-in-depth alongside the workflow-level filtering.
297-306: LGTM!The main execution block provides appropriate top-level error handling with detailed debugging output.
4c4291e to
6cf874a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @.github/scripts/ci_failure_bot.py:
- Around line 152-156: The code currently builds qa_checks_url and runtests_url
by hardcoding "master" which breaks repos using other default branches or
different layouts; update the logic that sets repo_url, qa_checks_url and
runtests_url (variables project_name, repo_url, qa_checks_url, runtests_url and
attribute self.repository_name) to first fetch the repository's default branch
via GitHub metadata (or accept a configurable default branch), then construct
the URLs using that branch name, and additionally verify whether the target
files exist in that branch (falling back to the repository root index or
omitting links if missing) so links are not incorrectly hardcoded to "master".
- Around line 77-79: Replace the current single-tail truncation ("logs":
response.text[-5000:]) with a first-N plus last-M strategy: define constants
(e.g., FIRST_CHARS and LAST_CHARS) whose sum equals the max payload (5000), then
build logs by taking response.text[:FIRST_CHARS] + ("\n...TRUNCATED...\n") +
response.text[-LAST_CHARS:] when response.text is longer than the limit; update
the dict key that currently uses response.text[-5000:] to use this combined
string and reuse similar logic already present around lines that implement the
PR diff approach (lines ~119-123) to keep formatting and truncation marker
consistent.
🧹 Nitpick comments (2)
.github/scripts/ci_failure_bot.py (2)
274-276: Consider posting a fallback comment when no build logs are found.Currently, if no build logs are retrieved, the bot exits silently without providing any feedback to the PR author. This might leave contributors confused about why the CI failed but no bot comment appeared.
Consider posting a minimal comment explaining that the analysis couldn't be performed and directing users to check the Actions tab.
💬 Suggested addition
if not build_logs: print("No build logs found") + self.post_comment( + "## CI Build Failed\n\n" + "Unable to retrieve build logs for automated analysis. " + "Please check the [Actions tab]" + f"(https://github.com/{self.repository_name}/actions/runs/{self.workflow_run_id}) " + "for details.\n\n" + "See: https://openwisp.io/docs/dev/developer/contributing.html" + ) return
291-293: Move the traceback import to the top of the file.The
tracebackmodule is imported inline here and again on line 303. It's a standard library module and should be imported at the top with other imports for consistency and clarity.import json import os import sys +import traceback import google.genai as genaiThen remove the inline imports on lines 291 and 303.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/scripts/ci_failure_bot.py.github/workflows/ci-failure-bot.ymldocs/developer/ci-failure-bot.rstdocs/user/ci-failure-bot.rst
🚧 Files skipped from review as they are similar to previous changes (3)
- docs/user/ci-failure-bot.rst
- docs/developer/ci-failure-bot.rst
- .github/workflows/ci-failure-bot.yml
🧰 Additional context used
🪛 Ruff (0.14.10)
.github/scripts/ci_failure_bot.py
94-94: Consider moving this statement to an else block
(TRY300)
202-202: Consider moving this statement to an else block
(TRY300)
203-203: Do not catch blind exception: Exception
(BLE001)
288-288: Do not catch blind exception: Exception
(BLE001)
301-301: Do not catch blind exception: Exception
(BLE001)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
- GitHub Check: Python==3.11 | django~=5.2.0
- GitHub Check: Python==3.13 | django~=5.1.0
- GitHub Check: Python==3.11 | django~=4.2.0
- GitHub Check: Python==3.11 | django~=5.0.0
- GitHub Check: Python==3.10 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=5.1.0
- GitHub Check: Python==3.13 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=5.0.0
- GitHub Check: Python==3.12 | django~=5.2.0
- GitHub Check: Python==3.11 | django~=5.1.0
- GitHub Check: Python==3.10 | django~=4.2.0
- GitHub Check: Python==3.12 | django~=4.2.0
- GitHub Check: Python==3.10 | django~=5.1.0
- GitHub Check: Python==3.10 | django~=5.0.0
🔇 Additional comments (4)
.github/scripts/ci_failure_bot.py (4)
99-134: Well-structured PR diff retrieval.The method handles errors appropriately and uses a reasonable truncation strategy (first + last) that preserves both context and final changes.
228-244: Excellent deduplication logic to prevent comment spam.The marker-based approach combined with bot login verification ensures only one bot comment exists per PR, and it gets updated on subsequent failures rather than creating duplicates.
53-54: The model namegemini-2.5-flashis valid and currently supported by the Gemini API with no issues.
200-205: Catch specific Gemini API exceptions instead of broadException.The broad
Exceptioncatch on line 203 is flagged by static analysis (BLE001). The google-genai SDK provides specific exception classes ingoogle.genai.errorsthat should be caught:APIError(base class),ClientError(4xx), andServerError(5xx). Import and catchAPIErrorto provide better error visibility and prevent masking unexpected issues while still handling SDK-specific failures.Likely an incorrect or invalid review comment.
6cf874a to
0a901ee
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @.github/scripts/ci_failure_bot.py:
- Line 10: The import at line 10 is wrong; confirm which Gemini SDK is installed
and replace `import google.genai as genai` with the correct import: use `from
google import genai` if using the new google-genai package, or `import
google.generativeai as genai` if using the legacy google-generativeai package,
and ensure any subsequent references to genai (e.g., model name
"gemini-2.5-flash") remain unchanged.
🧹 Nitpick comments (5)
.github/scripts/ci_failure_bot.py (5)
83-92: Consider including step logs for more precise diagnostics.The code collects metadata for failed steps (name, number) but not their actual logs. If step-level logs are available and would help with more precise failure analysis, consider fetching them similar to how job logs are retrieved.
94-97: Optional: Refactor exception handling structure.The static analysis tool suggests moving the
return []statement to anelseblock for consistency. While the current code is correct, the following structure may be slightly more explicit:♻️ Proposed refactor
- return build_logs - except (GithubException, ValueError) as e: - print(f"Error getting build logs: {e}") - return [] + except (GithubException, ValueError) as e: + print(f"Error getting build logs: {e}") + return [] + else: + return build_logs
165-198: Consider extracting the prompt template for maintainability.The AI prompt is extensive (30+ lines) and embedded directly in the code with multiple
noqacomments to bypass line-length checks. While this keeps the script self-contained, extracting the prompt to a separate template file or constant would improve:
- Maintainability: easier to iterate on prompt engineering
- Readability: cleaner code structure
- Testability: easier to test prompt variations
However, if keeping the script as a single file is a priority, the current approach is acceptable.
237-244: Consider caching bot login for efficiency.The bot's login is fetched via
self.github.get_user().loginon every call topost_comment. Since this value doesn't change, consider caching it in__init__:♻️ Proposed optimization
In
__init__, after line 50:self.github = Github(self.github_token) self.repo = self.github.get_repo(self.repository_name) self.bot_login = self.github.get_user().login # Cache bot loginThen in
post_comment, line 237:- bot_login = self.github.get_user().login + bot_login = self.bot_login
274-276: Consider posting a fallback comment when build logs are unavailable.When
build_logsis empty, the bot silently exits without posting any PR comment. This might happen if:
- Logs aren't ready yet (race condition with workflow_run events)
- There's a permission issue fetching logs
- The job hasn't actually failed despite triggering this workflow
Consider posting a minimal fallback comment to inform contributors:
💬 Proposed enhancement
if not build_logs: - print("No build logs found") - return + print("No build logs found, posting fallback comment") + fallback_msg = """ +## CI Build Failed + +Build logs are not yet available for analysis. Please check the CI logs directly or wait a moment for the automated analysis. + +If this persists, see: https://openwisp.io/docs/dev/developer/contributing.html +""" + self.post_comment(fallback_msg) + return
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/scripts/ci_failure_bot.py.github/workflows/ci-failure-bot.ymldocs/developer/ci-failure-bot.rstdocs/user/ci-failure-bot.rst
🚧 Files skipped from review as they are similar to previous changes (3)
- docs/developer/ci-failure-bot.rst
- .github/workflows/ci-failure-bot.yml
- docs/user/ci-failure-bot.rst
🧰 Additional context used
🪛 Ruff (0.14.10)
.github/scripts/ci_failure_bot.py
94-94: Consider moving this statement to an else block
(TRY300)
202-202: Consider moving this statement to an else block
(TRY300)
203-203: Do not catch blind exception: Exception
(BLE001)
288-288: Do not catch blind exception: Exception
(BLE001)
301-301: Do not catch blind exception: Exception
(BLE001)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
- GitHub Check: Python==3.11 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=5.2.0
- GitHub Check: Python==3.13 | django~=5.2.0
- GitHub Check: Python==3.11 | django~=4.2.0
- GitHub Check: Python==3.11 | django~=5.0.0
- GitHub Check: Python==3.13 | django~=5.1.0
- GitHub Check: Python==3.11 | django~=5.1.0
- GitHub Check: Python==3.12 | django~=5.1.0
- GitHub Check: Python==3.12 | django~=4.2.0
- GitHub Check: Python==3.12 | django~=5.0.0
- GitHub Check: Python==3.10 | django~=4.2.0
- GitHub Check: Python==3.10 | django~=5.2.0
- GitHub Check: Python==3.10 | django~=5.0.0
- GitHub Check: Python==3.10 | django~=5.1.0
🔇 Additional comments (8)
.github/scripts/ci_failure_bot.py (8)
99-133: LGTM!The PR diff retrieval logic is well-implemented:
- Proper validation of PR_NUMBER
- Good truncation strategy (first 4000 + last 4000 chars) to preserve context while managing token limits
- Appropriate error handling and timeout
- Secure authentication header usage
135-146: LGTM!The workflow YAML retrieval is straightforward and correct. Workflow files are typically small, so the lack of size limiting is acceptable.
200-205: Broad exception handling is acceptable here.The static analysis tool flags catching bare
Exception, but in this context it's appropriate because:
- This is a top-level API call where any failure should trigger the fallback
- The fallback provides graceful degradation
- Errors are logged before returning the fallback
The current implementation correctly prioritizes resilience over precise exception handling.
207-220: LGTM!The fallback response provides helpful, actionable guidance when AI analysis is unavailable. The OpenWISP-specific commands and URLs are consistent with the bot's intended use case.
238-244: LGTM! Deduplication logic is well-designed.The comment deduplication logic correctly:
- Uses an invisible HTML marker to identify bot comments
- Verifies both the comment author and marker presence
- Updates existing comments instead of creating duplicates
- Handles the case where no existing comment is found
This prevents comment spam on PRs with multiple CI failures.
258-267: LGTM! Dependabot skip logic is robust.The dependabot check correctly:
- Performs a case-insensitive check on the actor login
- Returns early to avoid unnecessary work
- Handles potential AttributeError if actor is None
- Logs the skip action for visibility
288-294: Top-level exception handling is appropriate.The static analysis tool flags catching bare
Exception, but this is exactly where comprehensive error handling is needed:
- This is the main orchestration method where any unhandled error should be caught
- The handler logs detailed debugging information (error type, traceback)
- Exiting with
sys.exit(1)properly signals failure to the CI systemThe current implementation is correct.
297-306: LGTM! Defense-in-depth error handling.The main block provides an additional layer of error handling that catches any exceptions during bot initialization that wouldn't be caught by the
run()method's error handler. The comprehensive error logging (message, type, traceback) and explicitsys.exit(1)are appropriate for a CI script.
| project_name = self.repository_name.split("/")[-1] | ||
| repo_url = f"https://github.com/{self.repository_name}" | ||
| qa_checks_url = f"{repo_url}/blob/master/openwisp-qa-check" | ||
| runtests_url = f"{repo_url}/blob/master/runtests" | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hardcoded OpenWISP-specific paths limit reusability.
The URLs for openwisp-qa-check and runtests are hardcoded and assume a specific repository structure:
- Line 154:
/blob/master/openwisp-qa-check - Line 155:
/blob/master/runtests
If this bot is intended exclusively for OpenWISP repositories, this is acceptable. However, if there's any intent to reuse this bot in other projects, consider:
- Making these paths configurable via environment variables
- Checking if files exist before including their URLs
- Making the prompt more generic
Based on the PR objectives mentioning "OpenWISP Utils" specifically, is this bot designed exclusively for OpenWISP repositories, or should it be more generic?
|
Hi @nemesifier , I followed the reusable GitHub utils documentation you linked and aligned both RST files to match that structure. I also validated them locally with docutils, fixed EOF newlines, and confirmed they render correctly. CI is now failing only on the ReStructuredText formatting step with “file could be reformatted”, and I’m not able to reproduce that behavior locally. It looks like there may be a specific RST formatter or configuration used in CI that I’m missing. Could you please let me know which tool or command is used to normalize RST files in OpenWISP, or if there’s a preferred way to auto-format these docs before committing? Once I know that, I’ll fix it right away and avoid further commit churn. Thanks. |
|
@Srinath0916 your environment most probably needs to be updated, eg: |
ae7e794 to
00b0f3b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @.github/scripts/ci_failure_bot.py:
- Around line 72-97: The requests.get call can raise requests.RequestException
which isn't currently caught; update the exception handling around the block
containing requests.get (the code that builds build_logs) to also catch
requests.RequestException by adding it to the except tuple (e.g., except
(GithubException, ValueError, requests.RequestException) as e) and ensure the
handler logs or prints the error and returns an empty list just like the
existing handler for GithubException/ValueError; reference the requests.get call
and the existing except block to locate where to add requests.RequestException.
In @.github/workflows/ci-failure-bot.yml:
- Around line 42-50: The CI script uses set -e which causes the shell to exit
immediately if python .github/scripts/ci_failure_bot.py fails, making the
subsequent exit_code capture and the if-block (the echo "ERROR: CI Failure Bot
failed..." and exit $exit_code) unreachable; fix by either removing set -e so
you can run the python command, capture its exit code in exit_code and keep the
existing if-block, or keep set -e and delete the redundant exit_code capture and
if-block (or replace it with a simple command chaining strategy to handle errors
explicitly).
🧹 Nitpick comments (2)
.github/scripts/ci_failure_bot.py (2)
288-294: Movetracebackimport to the top of the file.The
tracebackmodule is imported inside the exception handler. Moving it to the top with other imports follows Python conventions and avoids potential import failures during error handling.Suggested fix
At the top of the file (after line 8):
import tracebackThen update the exception handler:
except Exception as e: print(f"CRITICAL ERROR in CI Failure Bot: {e}") print(f"Error type: {type(e).__name__}") - import traceback - traceback.print_exc() sys.exit(1)
274-276: Consider posting a minimal comment when no build logs are found.Currently, the bot silently exits without posting a comment when build logs aren't available. This could leave contributors wondering why they didn't receive feedback. Consider posting a brief message directing them to check the CI logs manually.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/scripts/ci_failure_bot.py.github/workflows/ci-failure-bot.ymldocs/developer/ci-failure-bot.rstdocs/user/ci-failure-bot.rst
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/developer/ci-failure-bot.rst
🧰 Additional context used
🪛 Ruff (0.14.10)
.github/scripts/ci_failure_bot.py
94-94: Consider moving this statement to an else block
(TRY300)
202-202: Consider moving this statement to an else block
(TRY300)
203-203: Do not catch blind exception: Exception
(BLE001)
288-288: Do not catch blind exception: Exception
(BLE001)
301-301: Do not catch blind exception: Exception
(BLE001)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
- GitHub Check: Python==3.12 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=5.1.0
- GitHub Check: Python==3.13 | django~=5.2.0
- GitHub Check: Python==3.11 | django~=5.2.0
- GitHub Check: Python==3.11 | django~=5.1.0
- GitHub Check: Python==3.12 | django~=5.0.0
- GitHub Check: Python==3.10 | django~=5.0.0
- GitHub Check: Python==3.12 | django~=4.2.0
- GitHub Check: Python==3.11 | django~=4.2.0
- GitHub Check: Python==3.10 | django~=5.2.0
- GitHub Check: Python==3.10 | django~=4.2.0
- GitHub Check: Python==3.13 | django~=5.1.0
- GitHub Check: Python==3.11 | django~=5.0.0
- GitHub Check: Python==3.10 | django~=5.1.0
🔇 Additional comments (9)
docs/user/ci-failure-bot.rst (1)
1-95: Well-structured documentation with appropriate disclosures.The documentation clearly explains the CI Failure Bot's features, workflow, configuration, and limitations. The privacy and API cost considerations (lines 84-88) are appropriately disclosed, which is important for organizations evaluating this feature.
Minor note: Ensure the file ends with a trailing newline for RST consistency.
.github/scripts/ci_failure_bot.py (7)
16-54: LGTM!The initialization properly validates required environment variables with clear error messages and handles type conversion for
WORKFLOW_RUN_ID.
99-133: LGTM!Good handling of large diffs with truncation strategy, and comprehensive exception handling including
requests.RequestException.
135-146: LGTM!Clean implementation for retrieving workflow YAML content.
200-205: Broad exception catch is acceptable here for resilience.The static analyzer flags
except Exception(BLE001), but catching broadly here is reasonable since the Gemini SDK can raise various exception types, and graceful fallback tofallback_response()ensures the bot remains functional. Consider logging the exception type for debugging.
207-220: LGTM!Provides sensible fallback guidance when AI analysis is unavailable.
222-250: LGTM!Good implementation of comment deduplication using a marker. The approach of checking for existing bot comments before creating new ones prevents spam.
297-306: LGTM!Entry point with appropriate top-level exception handling. The same
tracebackimport suggestion from the previous comment applies here as well..github/workflows/ci-failure-bot.yml (1)
22-27: Bothactions/checkout@v6andactions/setup-python@v6are valid and current as of January 2026. No action needed.
| set -e # Exit immediately if any command fails | ||
| echo "Starting CI Failure Bot..." | ||
| python .github/scripts/ci_failure_bot.py | ||
| exit_code=$? | ||
| if [ $exit_code -ne 0 ]; then | ||
| echo "ERROR: CI Failure Bot failed with exit code $exit_code" | ||
| echo "This indicates a critical issue with the bot logic that needs immediate attention" | ||
| exit $exit_code | ||
| fi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unreachable error handling code due to set -e.
With set -e on line 42, the shell will exit immediately if python .github/scripts/ci_failure_bot.py fails, so lines 45-50 will never execute. Either remove set -e to allow capturing the exit code, or remove the redundant error handling.
Suggested fix: Simplify by relying on set -e
run: |
set -e # Exit immediately if any command fails
echo "Starting CI Failure Bot..."
python .github/scripts/ci_failure_bot.py
- exit_code=$?
- if [ $exit_code -ne 0 ]; then
- echo "ERROR: CI Failure Bot failed with exit code $exit_code"
- echo "This indicates a critical issue with the bot logic that needs immediate attention"
- exit $exit_code
- fi
echo "CI Failure Bot completed successfully"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| set -e # Exit immediately if any command fails | |
| echo "Starting CI Failure Bot..." | |
| python .github/scripts/ci_failure_bot.py | |
| exit_code=$? | |
| if [ $exit_code -ne 0 ]; then | |
| echo "ERROR: CI Failure Bot failed with exit code $exit_code" | |
| echo "This indicates a critical issue with the bot logic that needs immediate attention" | |
| exit $exit_code | |
| fi | |
| set -e # Exit immediately if any command fails | |
| echo "Starting CI Failure Bot..." | |
| python .github/scripts/ci_failure_bot.py | |
| echo "CI Failure Bot completed successfully" |
🤖 Prompt for AI Agents
In @.github/workflows/ci-failure-bot.yml around lines 42 - 50, The CI script
uses set -e which causes the shell to exit immediately if python
.github/scripts/ci_failure_bot.py fails, making the subsequent exit_code capture
and the if-block (the echo "ERROR: CI Failure Bot failed..." and exit
$exit_code) unreachable; fix by either removing set -e so you can run the python
command, capture its exit code in exit_code and keep the existing if-block, or
keep set -e and delete the redundant exit_code capture and if-block (or replace
it with a simple command chaining strategy to handle errors explicitly).
00b0f3b to
362f80c
Compare
| def analyze_with_gemini(self, build_logs, pr_diff, workflow_yaml): | ||
| """Send context to Gemini for intelligent analysis""" | ||
|
|
||
| # Prepare context for Gemini | ||
| project_name = self.repository_name.split("/")[-1] | ||
| repo_url = f"https://github.com/{self.repository_name}" | ||
| qa_checks_url = f"{repo_url}/blob/master/openwisp-qa-check" | ||
| runtests_url = f"{repo_url}/blob/master/runtests" | ||
|
|
||
| # Build the context string with proper line breaks | ||
| build_logs_json = json.dumps(build_logs, indent=2) | ||
| if pr_diff: | ||
| pr_diff_json = json.dumps(pr_diff, indent=2) | ||
| else: | ||
| pr_diff_json = "No PR associated" | ||
|
|
||
| # Gemini prompt - ignore line length for readability | ||
| context = f""" | ||
| ### ROLE | ||
| You are the "Automated Maintainer Gatekeeper." Your goal is to analyze Pull Request (PR) build failures and provide direct, technically accurate, and no-nonsense feedback to contributors. # noqa: E501 | ||
|
|
||
| ### INPUT CONTEXT PROVIDED | ||
| 1. **Build Output/Logs:** {build_logs_json} | ||
| 2. **YAML Workflow:** {workflow_yaml or "Not available"} | ||
| 3. **PR Diff:** {pr_diff_json} | ||
| 4. **Project Name:** {project_name} # noqa: E501 | ||
| 5. **Repository:** {repo_url} | ||
| 6. **run-qa-checks:** {qa_checks_url} # noqa: E501 | ||
| 7. **runtests:** {runtests_url} # noqa: E501 | ||
|
|
||
| ### TASK | ||
| Analyze the provided context to determine why the build failed. Categorize the failure and respond according to the "Tone Guidelines" below. # noqa: E501 | ||
|
|
||
| ### TONE GUIDELINES | ||
| - **Direct & Honest:** Do not use "fluff" or overly polite corporate language. | ||
| - **Firm Standards:** If a PR is low-effort, spammy, or fails to follow basic instructions, state that clearly. | ||
| - **Action-Oriented:** Provide the exact command or file change needed to fix the error, unless the PR is spammy, in which case we should just declare the PR as potential SPAM and ask maintainers to manually review it. # noqa: E501 | ||
|
|
||
| ### RESPONSE STRUCTURE | ||
| 1. **Status Summary:** A one-sentence blunt assessment of the failure. | ||
| 2. **Technical Diagnosis:** | ||
| - Identify the specific line/test that failed. | ||
| - Explain *why* it failed. | ||
| 3. **Required Action:** Provide a code block or specific steps the contributor must take. | ||
| 4. **Quality Warning (If Applicable):** If the PR appears to be "spam" (e.g., trivial README changes, AI-generated nonsense, or repeated basic errors), include a firm statement that such contributions are a drain on project resources and ping the maintainers asking them for manual review. # noqa: E501 | ||
|
|
||
| ### EXAMPLE RESPONSE STYLE | ||
| "The build failed because you neglected to update the test suite to match your logic changes. This project does not accept functional changes without corresponding test updates. Refer to the log at line 452. Update tests/logic_test.py before re-submitting. We prioritize high-quality, ready-to-merge code; please ensure you run local tests before pushing." # noqa: E501 | ||
|
|
||
| Analyze the failure and provide your response: | ||
| """ | ||
|
|
||
| try: | ||
| response = self.model.generate_content(context) | ||
| return response.text | ||
| except (ValueError, ConnectionError, Exception) as e: | ||
| print(f"Error calling Gemini API: {e}") | ||
| return self.fallback_response() | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚫 SKIP
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In @.github/scripts/ci_failure_bot.py:
- Around line 141-153: get_workflow_yaml currently fetches the workflow file
from the default branch using repo.get_contents(workflow_path), which can differ
from the commit that ran; change the call in get_workflow_yaml to request the
file at the run commit by passing ref=workflow_run.head_sha (i.e., use
repo.get_contents(workflow_path, ref=workflow_run.head_sha)) so the returned
YAML matches the workflow_run that executed.
- Around line 10-13: The script uses legacy calls genai.configure(...) and
genai.GenerativeModel(...) which are not present in google-genai; replace the
legacy API usage with the new Client API: instantiate client =
genai.Client(api_key=...) instead of genai.configure, and replace any
genai.GenerativeModel(...).generate(...) calls with
client.models.generate_content(...) (or client.models.generate(...) per SDK
version) and adapt parameters accordingly; ensure the import stays as from
google import genai and update any variable names (e.g., client) and error
handling around the new client.methods; alternatively, if you prefer to keep the
current code unchanged, update CI dependencies to install
google-generativeai>=0.5.0 so genai.configure and genai.GenerativeModel exist.
🧹 Nitpick comments (5)
.github/scripts/ci_failure_bot.py (5)
105-140: Diff fetch should handle non-200 responses explicitly (and avoid silent “None”).If diff fetch fails (403, 404, rate limit), you currently fall through and return
Nonewith only a print. Consider usingresponse.raise_for_status()and including the status code in the error message to make debugging in Actions logs easier.
206-212: Avoidexcept Exceptionhere; catch specific SDK/network errors + include actionable diagnostics.Catching
Exceptioncan hide programmer errors and make failures harder to debug. Prefer SDK-specific exceptions (plusrequests.RequestException-style ones if applicable) and log a short, structured reason before falling back. (Ruff BLE001/TRY300 hints are pointing at this area.)
228-257: Comment dedup is good; consider limiting comment scan to reduce API work on long PR threads.
pr.get_issue_comments()can be large; iterating all comments may add latency/rate-limit pressure. If PyGithub supports paging, consider scanning newest pages first or stopping early once you pass your own most recent marker.
258-283: If no logs are found, consider still posting a fallback comment (otherwise the bot is silent).Right now
run()returns early whenbuild_logsis empty. In practice, “no logs found” is itself something a contributor needs help with (permissions, API error, retention, etc.). Posting the fallback response (or a short diagnostic) would reduce confusion.
294-301: Two broad catch-alls (except Exception) likely overlap; keep one and ensure exit codes match desired workflow behavior.You catch broad exceptions in both
run()and__main__. Consider consolidating to one top-level handler so failures don’t get double-logged, and ensure you exit non-zero only when you want the workflow to be marked failed. (Ruff BLE001 flags apply here.)Also applies to: 307-312
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/scripts/ci_failure_bot.py.github/workflows/ci-failure-bot.ymldocs/developer/ci-failure-bot.rstdocs/user/ci-failure-bot.rst
🚧 Files skipped from review as they are similar to previous changes (3)
- docs/developer/ci-failure-bot.rst
- docs/user/ci-failure-bot.rst
- .github/workflows/ci-failure-bot.yml
🧰 Additional context used
🪛 Ruff (0.14.10)
.github/scripts/ci_failure_bot.py
100-100: Consider moving this statement to an else block
(TRY300)
208-208: Consider moving this statement to an else block
(TRY300)
209-209: Do not catch blind exception: Exception
(BLE001)
294-294: Do not catch blind exception: Exception
(BLE001)
307-307: Do not catch blind exception: Exception
(BLE001)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
- GitHub Check: Python==3.10 | django~=5.0.0
- GitHub Check: Python==3.10 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=5.0.0
- GitHub Check: Python==3.12 | django~=4.2.0
- GitHub Check: Python==3.11 | django~=4.2.0
- GitHub Check: Python==3.13 | django~=5.2.0
- GitHub Check: Python==3.11 | django~=5.0.0
- GitHub Check: Python==3.12 | django~=5.2.0
- GitHub Check: Python==3.11 | django~=5.1.0
- GitHub Check: Python==3.11 | django~=5.2.0
- GitHub Check: Python==3.10 | django~=4.2.0
- GitHub Check: Python==3.12 | django~=5.1.0
- GitHub Check: Python==3.10 | django~=5.1.0
- GitHub Check: Python==3.13 | django~=5.1.0
| def __init__(self): | ||
| self.github_token = os.environ.get("GITHUB_TOKEN") | ||
| self.gemini_api_key = os.environ.get("GEMINI_API_KEY") | ||
| self.workflow_run_id = os.environ.get("WORKFLOW_RUN_ID") | ||
| self.repository_name = os.environ.get("REPOSITORY") | ||
| self.pr_number = os.environ.get("PR_NUMBER") | ||
|
|
||
| if not all( | ||
| [ | ||
| self.github_token, | ||
| self.gemini_api_key, | ||
| self.workflow_run_id, | ||
| self.repository_name, | ||
| ] | ||
| ): | ||
| missing = [] | ||
| if not self.github_token: | ||
| missing.append("GITHUB_TOKEN") | ||
| if not self.gemini_api_key: | ||
| missing.append("GEMINI_API_KEY") | ||
| if not self.workflow_run_id: | ||
| missing.append("WORKFLOW_RUN_ID") | ||
| if not self.repository_name: | ||
| missing.append("REPOSITORY") | ||
| print(f"Missing required environment variables: {', '.join(missing)}") | ||
| sys.exit(1) | ||
|
|
||
| try: | ||
| self.workflow_run_id = int(self.workflow_run_id) | ||
| except ValueError: | ||
| print("Invalid WORKFLOW_RUN_ID: must be numeric") | ||
| sys.exit(1) | ||
|
|
||
| self.github = Github(self.github_token) | ||
| self.repo = self.github.get_repo(self.repository_name) | ||
|
|
||
| genai.configure(api_key=self.gemini_api_key) | ||
| model_name = os.environ.get("GEMINI_MODEL", "gemini-2.5-flash") | ||
| self.model = genai.GenerativeModel(model_name) | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid hard-failing when GEMINI_API_KEY is missing; allow “static-only” mode.
Right now the bot exits if GEMINI_API_KEY is absent, even though you have a fallback_response() path. Consider allowing the bot to run and post the fallback guidance when AI is not configured (especially useful for maintainers who don’t want org-level secrets yet).
Proposed change (make Gemini optional)
class CIFailureBot:
def __init__(self):
self.github_token = os.environ.get("GITHUB_TOKEN")
- self.gemini_api_key = os.environ.get("GEMINI_API_KEY")
+ self.gemini_api_key = os.environ.get("GEMINI_API_KEY")
self.workflow_run_id = os.environ.get("WORKFLOW_RUN_ID")
self.repository_name = os.environ.get("REPOSITORY")
self.pr_number = os.environ.get("PR_NUMBER")
if not all(
[
self.github_token,
- self.gemini_api_key,
self.workflow_run_id,
self.repository_name,
]
):
@@
- if not self.gemini_api_key:
- missing.append("GEMINI_API_KEY")
if not self.workflow_run_id:
missing.append("WORKFLOW_RUN_ID")
if not self.repository_name:
missing.append("REPOSITORY")
print(f"Missing required environment variables: {', '.join(missing)}")
sys.exit(1)
@@
self.github = Github(self.github_token)
self.repo = self.github.get_repo(self.repository_name)
- genai.configure(api_key=self.gemini_api_key)
- model_name = os.environ.get("GEMINI_MODEL", "gemini-2.5-flash")
- self.model = genai.GenerativeModel(model_name)
+ self.model = None
+ if self.gemini_api_key:
+ genai.configure(api_key=self.gemini_api_key)
+ model_name = os.environ.get("GEMINI_MODEL", "gemini-2.5-flash")
+ self.model = genai.GenerativeModel(model_name)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def __init__(self): | |
| self.github_token = os.environ.get("GITHUB_TOKEN") | |
| self.gemini_api_key = os.environ.get("GEMINI_API_KEY") | |
| self.workflow_run_id = os.environ.get("WORKFLOW_RUN_ID") | |
| self.repository_name = os.environ.get("REPOSITORY") | |
| self.pr_number = os.environ.get("PR_NUMBER") | |
| if not all( | |
| [ | |
| self.github_token, | |
| self.gemini_api_key, | |
| self.workflow_run_id, | |
| self.repository_name, | |
| ] | |
| ): | |
| missing = [] | |
| if not self.github_token: | |
| missing.append("GITHUB_TOKEN") | |
| if not self.gemini_api_key: | |
| missing.append("GEMINI_API_KEY") | |
| if not self.workflow_run_id: | |
| missing.append("WORKFLOW_RUN_ID") | |
| if not self.repository_name: | |
| missing.append("REPOSITORY") | |
| print(f"Missing required environment variables: {', '.join(missing)}") | |
| sys.exit(1) | |
| try: | |
| self.workflow_run_id = int(self.workflow_run_id) | |
| except ValueError: | |
| print("Invalid WORKFLOW_RUN_ID: must be numeric") | |
| sys.exit(1) | |
| self.github = Github(self.github_token) | |
| self.repo = self.github.get_repo(self.repository_name) | |
| genai.configure(api_key=self.gemini_api_key) | |
| model_name = os.environ.get("GEMINI_MODEL", "gemini-2.5-flash") | |
| self.model = genai.GenerativeModel(model_name) | |
| def __init__(self): | |
| self.github_token = os.environ.get("GITHUB_TOKEN") | |
| self.gemini_api_key = os.environ.get("GEMINI_API_KEY") | |
| self.workflow_run_id = os.environ.get("WORKFLOW_RUN_ID") | |
| self.repository_name = os.environ.get("REPOSITORY") | |
| self.pr_number = os.environ.get("PR_NUMBER") | |
| if not all( | |
| [ | |
| self.github_token, | |
| self.workflow_run_id, | |
| self.repository_name, | |
| ] | |
| ): | |
| missing = [] | |
| if not self.github_token: | |
| missing.append("GITHUB_TOKEN") | |
| if not self.workflow_run_id: | |
| missing.append("WORKFLOW_RUN_ID") | |
| if not self.repository_name: | |
| missing.append("REPOSITORY") | |
| print(f"Missing required environment variables: {', '.join(missing)}") | |
| sys.exit(1) | |
| try: | |
| self.workflow_run_id = int(self.workflow_run_id) | |
| except ValueError: | |
| print("Invalid WORKFLOW_RUN_ID: must be numeric") | |
| sys.exit(1) | |
| self.github = Github(self.github_token) | |
| self.repo = self.github.get_repo(self.repository_name) | |
| self.model = None | |
| if self.gemini_api_key: | |
| genai.configure(api_key=self.gemini_api_key) | |
| model_name = os.environ.get("GEMINI_MODEL", "gemini-2.5-flash") | |
| self.model = genai.GenerativeModel(model_name) |
| def analyze_with_gemini(self, build_logs, pr_diff, workflow_yaml): | ||
| """Send context to Gemini for intelligent analysis""" | ||
|
|
||
| # Prepare context for Gemini | ||
| project_name = self.repository_name.split("/")[-1] | ||
| repo_url = f"https://github.com/{self.repository_name}" | ||
| qa_checks_url = f"{repo_url}/blob/master/openwisp-qa-check" | ||
| runtests_url = f"{repo_url}/blob/master/runtests" | ||
|
|
||
| # Build the context string with proper line breaks | ||
| build_logs_json = json.dumps(build_logs, indent=2) | ||
| if pr_diff: | ||
| pr_diff_json = json.dumps(pr_diff, indent=2) | ||
| else: | ||
| pr_diff_json = "No PR associated" | ||
|
|
||
| # Gemini prompt - ignore line length for readability | ||
| context = f""" | ||
| ### ROLE | ||
| You are the "Automated Maintainer Gatekeeper." Your goal is to analyze Pull Request (PR) build failures and provide direct, technically accurate, and no-nonsense feedback to contributors. # noqa: E501 | ||
|
|
||
| ### INPUT CONTEXT PROVIDED | ||
| 1. **Build Output/Logs:** {build_logs_json} | ||
| 2. **YAML Workflow:** {workflow_yaml or "Not available"} | ||
| 3. **PR Diff:** {pr_diff_json} | ||
| 4. **Project Name:** {project_name} # noqa: E501 | ||
| 5. **Repository:** {repo_url} | ||
| 6. **run-qa-checks:** {qa_checks_url} # noqa: E501 | ||
| 7. **runtests:** {runtests_url} # noqa: E501 | ||
|
|
||
| ### TASK | ||
| Analyze the provided context to determine why the build failed. Categorize the failure and respond according to the "Tone Guidelines" below. # noqa: E501 | ||
|
|
||
| ### TONE GUIDELINES | ||
| - **Direct & Honest:** Do not use "fluff" or overly polite corporate language. | ||
| - **Firm Standards:** If a PR is low-effort, spammy, or fails to follow basic instructions, state that clearly. | ||
| - **Action-Oriented:** Provide the exact command or file change needed to fix the error, unless the PR is spammy, in which case we should just declare the PR as potential SPAM and ask maintainers to manually review it. # noqa: E501 | ||
|
|
||
| ### RESPONSE STRUCTURE | ||
| 1. **Status Summary:** A one-sentence blunt assessment of the failure. | ||
| 2. **Technical Diagnosis:** | ||
| - Identify the specific line/test that failed. | ||
| - Explain *why* it failed. | ||
| 3. **Required Action:** Provide a code block or specific steps the contributor must take. | ||
| 4. **Quality Warning (If Applicable):** If the PR appears to be "spam" (e.g., trivial README changes, AI-generated nonsense, or repeated basic errors), include a firm statement that such contributions are a drain on project resources and ping the maintainers asking them for manual review. # noqa: E501 | ||
|
|
||
| ### EXAMPLE RESPONSE STYLE | ||
| "The build failed because you neglected to update the test suite to match your logic changes. This project does not accept functional changes without corresponding test updates. Refer to the log at line 452. Update tests/logic_test.py before re-submitting. We prioritize high-quality, ready-to-merge code; please ensure you run local tests before pushing." # noqa: E501 | ||
|
|
||
| Analyze the failure and provide your response: | ||
| """ | ||
|
|
||
| try: | ||
| response = self.model.generate_content(context) | ||
| return response.text | ||
| except (ValueError, ConnectionError, Exception) as e: | ||
| print(f"Error calling Gemini API: {e}") | ||
| return self.fallback_response() | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
fd "ci_failure_bot.py" --type fRepository: openwisp/openwisp-utils
Length of output: 110
🏁 Script executed:
cat -n .github/scripts/ci_failure_bot.pyRepository: openwisp/openwisp-utils
Length of output: 14542
🏁 Script executed:
fd "\.yml|\.yaml" --path "*/.github/workflows/*" --type fRepository: openwisp/openwisp-utils
Length of output: 282
🏁 Script executed:
rg "ci_failure_bot" --type yaml --type ymlRepository: openwisp/openwisp-utils
Length of output: 94
🏁 Script executed:
find .github/workflows -name "*.yaml" -o -name "*.yml" 2>/dev/null | head -20Repository: openwisp/openwisp-utils
Length of output: 238
🏁 Script executed:
rg "ci_failure_bot" --type yamlRepository: openwisp/openwisp-utils
Length of output: 215
🏁 Script executed:
cat -n .github/workflows/ci-failure-bot.ymlRepository: openwisp/openwisp-utils
Length of output: 2009
🏁 Script executed:
cat -n .github/workflows/ci.yml | head -50Repository: openwisp/openwisp-utils
Length of output: 1570
🏁 Script executed:
rg "fork\|ENABLE_AI" .github/scripts/ --type pyRepository: openwisp/openwisp-utils
Length of output: 49
🏁 Script executed:
rg "github.event.pull_request.head.repository.fork" .github/workflows/ || echo "Fork check not found"Repository: openwisp/openwisp-utils
Length of output: 87
🏁 Script executed:
python3 << 'EOF'
# Let's calculate the worst case prompt size
logs_truncation = 2000 + 3000 # 5000 chars
diff_truncation = 4000 + 4000 # 8000 chars
# Estimate JSON expansion with indent=2
logs_json_overhead = logs_truncation * 1.15 # ~15% overhead for JSON formatting
diff_json_overhead = diff_truncation * 1.15
# The prompt template
prompt_base = """
### ROLE
You are the "Automated Maintainer Gatekeeper."...
[Full prompt template]
"""
# Rough calculation
print(f"Logs JSON expansion: ~{int(logs_json_overhead)} chars")
print(f"Diff JSON expansion: ~{int(diff_json_overhead)} chars")
print(f"Prompt base template: {len(prompt_base)} chars")
print(f"\nEstimated max prompt without size limit: {int(logs_json_overhead + diff_json_overhead + 2000)} chars")
print("\nNote: No explicit total prompt size cap in code")
EOFRepository: openwisp/openwisp-utils
Length of output: 266
Remove or gate AI analysis for fork PRs and add explicit opt-in control.
This code sends CI logs and PR diffs to an external Gemini API without adequate safeguards:
- No fork detection: The workflow runs on all PRs including forks. Add a check for
github.event.pull_request.head.repository.forkto skip external PRs entirely. - No opt-in flag: AI analysis runs by default. Add an
ENABLE_AI_ANALYSISenv var check (disabled by default) to allow explicit enablement per repository. - No secret redaction: Code relies only on GitHub's built-in secret masking; add explicit redaction of common patterns (API tokens, private keys) before sending data.
- No hard prompt size limit: While individual sections are truncated (logs 5000 chars, diffs 8000 chars), there's no hard cap on total prompt size. Add an explicit maximum.
- Ineffective noqa comments: Lines 173, 179, 181, 185, 190, 201 contain
# noqa: E501inside the f-string—these are sent as content to Gemini and have no effect on linters. Remove them.
🧰 Tools
🪛 Ruff (0.14.10)
208-208: Consider moving this statement to an else block
(TRY300)
209-209: Do not catch blind exception: Exception
(BLE001)
362f80c to
adaa0f9
Compare
…p#524 - Add complete test suite with comprehensive coverage for all bot functions - Add user and developer documentation following OpenWISP structure - Improve error handling in workflow to fail loudly on script errors - Address CodeRabbit feedback on exception handling - Follow OpenWISP documentation patterns with Inputs, Usage Example, and Note sections - Ensure proper QA formatting with noqa directives for long Gemini prompt lines Closes openwisp#524
adaa0f9 to
9a64ed9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.github/scripts/ci_failure_bot.py:
- Around line 168-230: The Gemini prompt in analyze_with_gemini contains inline
"# noqa: E501" comments inside the f-string (the context variable) which will be
sent to the model; remove those markers from the multi-line string (delete all
occurrences of "# noqa: E501" inside the context) and, if flake8 complains about
line length, move the lint directive outside the string (for example add a
single "# noqa: E501" on the def analyze_with_gemini(...) line or add a
flake8/pep8 disable comment immediately above the method) so the prompt text
remains clean while preserving lint suppression for the method as needed; no
code changes to exception handling are required (keep the existing broad
Exception handling).
🧹 Nitpick comments (5)
.github/workflows/ci-failure-bot.yml (3)
18-18: Consider using exact match for dependabot check.The
contains()check could inadvertently match usernames like "my-dependabot-test". GitHub's dependabot typically uses exact logins likedependabot[bot].💡 Consider more precise matching
- if: ${{ github.event.workflow_run.conclusion == 'failure' && !contains(github.event.workflow_run.actor.login, 'dependabot') }} + if: ${{ github.event.workflow_run.conclusion == 'failure' && github.event.workflow_run.actor.login != 'dependabot[bot]' }}Alternatively, if you need to handle multiple bot variations:
if: ${{ github.event.workflow_run.conclusion == 'failure' && !startsWith(github.event.workflow_run.actor.login, 'dependabot') }}
29-32: Consider pinning dependency versions for reproducibility.Using
>=constraints allows dependency versions to drift over time, potentially introducing breaking changes. For automation scripts, pinned versions provide more predictable behavior.📌 Example with pinned versions
- name: Install dependencies - # Install minimum required versions run: | - pip install requests>=2.32.5 PyGithub>=2.0.0 google-genai>=0.2.0 + pip install requests==2.32.5 PyGithub==2.5.0 google-genai==0.2.2Note: You'll need to verify the latest stable versions for PyGithub and google-genai.
34-51: Script execution is well-structured.The environment variable setup and error handling provide good observability. The fallback for PR_NUMBER correctly handles non-PR workflow runs.
Minor note: Lines 45-50 check and propagate the exit code, but
set -eon line 42 already ensures immediate exit on failure. This redundancy adds clarity but could be simplified if preferred..github/scripts/ci_failure_bot.py (2)
58-117: Build log retrieval is well-implemented with proper ZIP handling.The ZIP archive detection (PK signature) and extraction, truncation strategy for large logs, and error handling are all appropriate. The authentication headers and timeout on requests are good practices.
Minor observation: Lines 104-112 collect failed step metadata separately from the full job logs retrieved earlier. Since the full job logs already contain step output, the separate step metadata collection provides job_name/step_name/step_number but no additional log content. Consider whether this metadata duplication adds value to the Gemini analysis.
277-331: Main execution flow is well-structured with good security checks.The security measures to skip dependabot and fork PRs before sending code to external AI are appropriate. Early exits prevent unnecessary API calls, and the comprehensive error handling ensures failures are visible.
Note on fork PR check: Line 300-301 catches exceptions during fork status verification and continues execution. If the fork check fails, the bot proceeds with analysis, potentially sending external code to Gemini. While the workflow-level filtering and dependabot check provide defense-in-depth, consider whether an exception during fork verification should trigger a safer default (skip analysis).
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/scripts/ci_failure_bot.py.github/workflows/ci-failure-bot.ymldocs/developer/ci-failure-bot.rstdocs/user/ci-failure-bot.rst
🚧 Files skipped from review as they are similar to previous changes (2)
- docs/user/ci-failure-bot.rst
- docs/developer/ci-failure-bot.rst
🧰 Additional context used
🪛 Ruff (0.14.10)
.github/scripts/ci_failure_bot.py
114-114: Consider moving this statement to an else block
(TRY300)
227-227: Consider moving this statement to an else block
(TRY300)
228-228: Do not catch blind exception: Exception
(BLE001)
325-325: Do not catch blind exception: Exception
(BLE001)
338-338: Do not catch blind exception: Exception
(BLE001)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
- GitHub Check: Python==3.11 | django~=5.1.0
- GitHub Check: Python==3.10 | django~=5.2.0
- GitHub Check: Python==3.11 | django~=5.2.0
- GitHub Check: Python==3.10 | django~=4.2.0
- GitHub Check: Python==3.12 | django~=5.0.0
- GitHub Check: Python==3.12 | django~=5.2.0
- GitHub Check: Python==3.13 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=5.1.0
- GitHub Check: Python==3.13 | django~=5.1.0
- GitHub Check: Python==3.12 | django~=4.2.0
- GitHub Check: Python==3.10 | django~=5.0.0
- GitHub Check: Python==3.10 | django~=5.1.0
- GitHub Check: Python==3.11 | django~=4.2.0
- GitHub Check: Python==3.11 | django~=5.0.0
🔇 Additional comments (8)
.github/workflows/ci-failure-bot.yml (1)
1-13: LGTM: Workflow trigger and permissions are correctly configured.The workflow_run trigger on completion and the permission scope (write for issues/PRs, read for contents) are appropriate for the bot's functionality.
.github/scripts/ci_failure_bot.py (7)
1-14: LGTM: Imports are clean and necessary.All imported modules are used appropriately throughout the script.
119-153: LGTM: PR diff retrieval with appropriate truncation.The method correctly handles missing PR numbers, authenticates requests, and applies sensible truncation for large diffs. Error handling gracefully returns None when the PR cannot be accessed.
155-166: LGTM: Workflow YAML retrieval is correct.Fetching the workflow file at the specific commit (head_sha) ensures accuracy. Error handling is appropriate.
232-245: LGTM: Fallback response provides useful guidance.The static response gives contributors actionable steps when AI analysis is unavailable.
247-275: LGTM: Comment deduplication prevents spam.The marker-based deduplication strategy correctly identifies and updates existing bot comments rather than creating duplicates. This is a good user experience.
334-343: LGTM: Top-level error handling ensures visible failures.The catch-all exception handler at the entry point is appropriate for a CI script, ensuring any unexpected errors are logged and propagate as non-zero exit codes.
18-56: Initialization logic is well-structured with good validation.The environment variable validation provides clear error messages, and optional PR_NUMBER handling is correct. The numeric validation for WORKFLOW_RUN_ID prevents downstream type errors. The default Gemini model
gemini-2.5-flashis valid and current.
| def analyze_with_gemini(self, build_logs, pr_diff, workflow_yaml): | ||
| """Send context to Gemini for intelligent analysis""" | ||
|
|
||
| # Prepare context for Gemini | ||
| project_name = self.repository_name.split("/")[-1] | ||
| repo_url = f"https://github.com/{self.repository_name}" | ||
| qa_checks_url = f"{repo_url}/blob/master/openwisp-qa-check" | ||
| runtests_url = f"{repo_url}/blob/master/runtests" | ||
|
|
||
| # Build the context string with proper line breaks | ||
| build_logs_json = json.dumps(build_logs, indent=2) | ||
| if pr_diff: | ||
| pr_diff_json = json.dumps(pr_diff, indent=2) | ||
| else: | ||
| pr_diff_json = "No PR associated" | ||
|
|
||
| # Gemini prompt - ignore line length for readability | ||
| context = f""" | ||
| ### ROLE | ||
| You are the "Automated Maintainer Gatekeeper." Your goal is to analyze Pull Request (PR) build failures and provide direct, technically accurate, and no-nonsense feedback to contributors. # noqa: E501 | ||
|
|
||
| ### INPUT CONTEXT PROVIDED | ||
| 1. **Build Output/Logs:** {build_logs_json} | ||
| 2. **YAML Workflow:** {workflow_yaml or "Not available"} | ||
| 3. **PR Diff:** {pr_diff_json} | ||
| 4. **Project Name:** {project_name} # noqa: E501 | ||
| 5. **Repository:** {repo_url} | ||
| 6. **run-qa-checks:** {qa_checks_url} # noqa: E501 | ||
| 7. **runtests:** {runtests_url} # noqa: E501 | ||
|
|
||
| ### TASK | ||
| Analyze the provided context to determine why the build failed. Categorize the failure and respond according to the "Tone Guidelines" below. # noqa: E501 | ||
|
|
||
| ### TONE GUIDELINES | ||
| - **Direct & Honest:** Do not use "fluff" or overly polite corporate language. | ||
| - **Firm Standards:** If a PR is low-effort, spammy, or fails to follow basic instructions, state that clearly. | ||
| - **Action-Oriented:** Provide the exact command or file change needed to fix the error, unless the PR is spammy, in which case we should just declare the PR as potential SPAM and ask maintainers to manually review it. # noqa: E501 | ||
|
|
||
| ### RESPONSE STRUCTURE | ||
| 1. **Status Summary:** A one-sentence blunt assessment of the failure. | ||
| 2. **Technical Diagnosis:** | ||
| - Identify the specific line/test that failed. | ||
| - Explain *why* it failed. | ||
| 3. **Required Action:** Provide a code block or specific steps the contributor must take. | ||
| 4. **Quality Warning (If Applicable):** If the PR appears to be "spam" (e.g., trivial README changes, AI-generated nonsense, or repeated basic errors), include a firm statement that such contributions are a drain on project resources and ping the maintainers asking them for manual review. # noqa: E501 | ||
|
|
||
| ### EXAMPLE RESPONSE STYLE | ||
| "The build failed because you neglected to update the test suite to match your logic changes. This project does not accept functional changes without corresponding test updates. Refer to the log at line 452. Update tests/logic_test.py before re-submitting. We prioritize high-quality, ready-to-merge code; please ensure you run local tests before pushing." # noqa: E501 | ||
|
|
||
| Analyze the failure and provide your response: | ||
| """ | ||
|
|
||
| try: | ||
| # Use new Gemini client API | ||
| from google.genai import types | ||
| response = self.client.models.generate_content( | ||
| model=self.model_name, | ||
| contents=types.Part.from_text(context) | ||
| ) | ||
| return response.text | ||
| except (ValueError, ConnectionError, Exception) as e: | ||
| print(f"Error calling Gemini API: {e}") | ||
| return self.fallback_response() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove code quality markers from the Gemini prompt.
Lines 187, 193, 194, 195, 196, 199, 204, 212, 213, 215 contain # noqa: E501 comments embedded within the multi-line string that forms the Gemini prompt. These linting directives will be sent to the AI as part of the prompt text, which is likely unintentional.
🧹 Proposed fix to remove noqa comments from prompt
# Gemini prompt - ignore line length for readability
context = f"""
### ROLE
-You are the "Automated Maintainer Gatekeeper." Your goal is to analyze Pull Request (PR) build failures and provide direct, technically accurate, and no-nonsense feedback to contributors. # noqa: E501
+You are the "Automated Maintainer Gatekeeper." Your goal is to analyze Pull Request (PR) build failures and provide direct, technically accurate, and no-nonsense feedback to contributors.
### INPUT CONTEXT PROVIDED
1. **Build Output/Logs:** {build_logs_json}
2. **YAML Workflow:** {workflow_yaml or "Not available"}
3. **PR Diff:** {pr_diff_json}
-4. **Project Name:** {project_name} # noqa: E501
+4. **Project Name:** {project_name}
5. **Repository:** {repo_url}
-6. **run-qa-checks:** {qa_checks_url} # noqa: E501
-7. **runtests:** {runtests_url} # noqa: E501
+6. **run-qa-checks:** {qa_checks_url}
+7. **runtests:** {runtests_url}
### TASK
-Analyze the provided context to determine why the build failed. Categorize the failure and respond according to the "Tone Guidelines" below. # noqa: E501
+Analyze the provided context to determine why the build failed. Categorize the failure and respond according to the "Tone Guidelines" below.
### TONE GUIDELINES
- **Direct & Honest:** Do not use "fluff" or overly polite corporate language.
- **Firm Standards:** If a PR is low-effort, spammy, or fails to follow basic instructions, state that clearly.
-- **Action-Oriented:** Provide the exact command or file change needed to fix the error, unless the PR is spammy, in which case we should just declare the PR as potential SPAM and ask maintainers to manually review it. # noqa: E501
+- **Action-Oriented:** Provide the exact command or file change needed to fix the error, unless the PR is spammy, in which case we should just declare the PR as potential SPAM and ask maintainers to manually review it.
### RESPONSE STRUCTURE
1. **Status Summary:** A one-sentence blunt assessment of the failure.
@@ -210,11 +210,11 @@
- Explain *why* it failed.
3. **Required Action:** Provide a code block or specific steps the contributor must take.
-4. **Quality Warning (If Applicable):** If the PR appears to be "spam" (e.g., trivial README changes, AI-generated nonsense, or repeated basic errors), include a firm statement that such contributions are a drain on project resources and ping the maintainers asking them for manual review. # noqa: E501
+4. **Quality Warning (If Applicable):** If the PR appears to be "spam" (e.g., trivial README changes, AI-generated nonsense, or repeated basic errors), include a firm statement that such contributions are a drain on project resources and ping the maintainers asking them for manual review.
### EXAMPLE RESPONSE STYLE
-"The build failed because you neglected to update the test suite to match your logic changes. This project does not accept functional changes without corresponding test updates. Refer to the log at line 452. Update tests/logic_test.py before re-submitting. We prioritize high-quality, ready-to-merge code; please ensure you run local tests before pushing." # noqa: E501
+"The build failed because you neglected to update the test suite to match your logic changes. This project does not accept functional changes without corresponding test updates. Refer to the log at line 452. Update tests/logic_test.py before re-submitting. We prioritize high-quality, ready-to-merge code; please ensure you run local tests before pushing."
Analyze the failure and provide your response:
"""If the long lines cause linting issues, you can disable E501 for the entire method or the specific line range outside the string literal.
Regarding broad exception handling: The catch-all Exception on line 228 is flagged by static analysis but is acceptable here since it ensures a fallback response when Gemini fails. The bot's resilience is more important than strict exception specificity in this context.
🧰 Tools
🪛 Ruff (0.14.10)
227-227: Consider moving this statement to an else block
(TRY300)
228-228: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
In @.github/scripts/ci_failure_bot.py around lines 168 - 230, The Gemini prompt
in analyze_with_gemini contains inline "# noqa: E501" comments inside the
f-string (the context variable) which will be sent to the model; remove those
markers from the multi-line string (delete all occurrences of "# noqa: E501"
inside the context) and, if flake8 complains about line length, move the lint
directive outside the string (for example add a single "# noqa: E501" on the def
analyze_with_gemini(...) line or add a flake8/pep8 disable comment immediately
above the method) so the prompt text remains clean while preserving lint
suppression for the method as needed; no code changes to exception handling are
required (keep the existing broad Exception handling).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In @.github/scripts/ci_failure_bot.py:
- Around line 189-222: The prompt string assigned to the variable context in
.github/scripts/ci_failure_bot.py includes inline linter comments "# noqa: E501"
which will be sent to the LLM; remove all occurrences of these comments from the
f-string so the runtime prompt is clean. Edit the context f-string (the
multi-line prompt) to delete every "# noqa: E501" token (e.g., after the ROLE
line and any lines referencing project_name, repo_url, qa_checks_url,
runtests_url, and RESPONSE STRUCTURE) so the content sent to Gemini contains
only the intended prompt text.
- Line 56: The default Gemini model fallback self.model_name =
os.environ.get("GEMINI_MODEL", "gemini-2.5-flash") will retire on 2026-06-17;
update the fallback to a currently supported successor (e.g., a Gemini 3 family
model or a later 2.x GA model) and ensure GEMINI_MODEL remains overridable via
env; change the default string used in the os.environ.get call (symbol:
self.model_name) to the new model name, and add a short code comment referencing
the migration/deprecation date so future maintainers know why the default was
updated.
- Around line 296-306: The current fork check accesses pr.head.repo.full_name
which will raise when pr.head.repo is None for deleted fork repositories; update
the block that handles self.pr_number/pr to explicitly check if pr.head.repo is
None before accessing full_name and treat that case as a deleted fork (print a
clear message and return/skip), otherwise compare pr.head.repo.full_name to
self.repository_name as before; keep the existing except (GithubException,
ValueError) handling for API errors but avoid relying on it for the deleted-repo
case by performing the explicit None check on pr.head.repo.
🧹 Nitpick comments (3)
.github/scripts/ci_failure_bot.py (3)
18-57: Consider validating PR_NUMBER during initialization.While
PR_NUMBERis optional, if it's provided, validate it's numeric during initialization rather than catchingValueErrorlater in multiple methods (lines 127, 262, 299). This follows the fail-fast principle.♻️ Proposed refactor to validate PR_NUMBER early
try: self.workflow_run_id = int(self.workflow_run_id) except ValueError: print("Invalid WORKFLOW_RUN_ID: must be numeric") sys.exit(1) + + if self.pr_number and self.pr_number.strip(): + try: + self.pr_number = int(self.pr_number) + except ValueError: + print("Invalid PR_NUMBER: must be numeric") + sys.exit(1) + else: + self.pr_number = None
58-119: Consider restructuring the build logs data model.The method appends both full job logs (with
logsfield) and individual failed step metadata (withstep_name,step_numberfields) to the same list, resulting in inconsistent dictionary structures. This mixed format could complicate downstream processing.Consider either:
- Nesting step metadata under job entries
- Separating logs and step metadata into distinct lists
- Ensuring all entries have a consistent schema
♻️ Example of consistent schema
build_logs.append( { "job_name": job.name, "logs": log_text, + "failed_steps": [] } ) # Also get step details + failed_steps = [] for step in job.steps: if step.conclusion == "failure": - build_logs.append( - { - "job_name": job.name, - "step_name": step.name, - "step_number": step.number, - } - ) + failed_steps.append({ + "step_name": step.name, + "step_number": step.number, + }) + if failed_steps: + # Add to the last job entry + if build_logs and build_logs[-1]["job_name"] == job.name: + build_logs[-1]["failed_steps"] = failed_steps
178-179: Verify hard-coded file paths and consider the tone of AI responses.Two observations:
Hard-coded paths: Lines 178-179 assume
openwisp-qa-checkandruntestsexist at the repository root on themasterbranch. If these files are missing, moved, or on a different branch, the links will break.Aggressive tone: The prompt instructs the AI to be "blunt," "firm," and call out "spam" PRs. While accountability is important, this tone may discourage genuine contributors, especially newcomers who make honest mistakes. Open source communities typically benefit from being welcoming and educational.
Consider:
- Dynamically checking if these files exist before including links
- Moderating the tone to be direct but constructive rather than harsh
- Getting community feedback on whether this tone aligns with OpenWISP's contributor culture
Also applies to: 205-210
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/scripts/ci_failure_bot.py.github/workflows/ci-failure-bot.ymldocs/developer/ci-failure-bot.rstdocs/user/ci-failure-bot.rst
🚧 Files skipped from review as they are similar to previous changes (3)
- docs/user/ci-failure-bot.rst
- .github/workflows/ci-failure-bot.yml
- docs/developer/ci-failure-bot.rst
🧰 Additional context used
🪛 Ruff (0.14.10)
.github/scripts/ci_failure_bot.py
116-116: Consider moving this statement to an else block
(TRY300)
231-231: Consider moving this statement to an else block
(TRY300)
232-232: Do not catch blind exception: Exception
(BLE001)
329-329: Do not catch blind exception: Exception
(BLE001)
342-342: Do not catch blind exception: Exception
(BLE001)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
- GitHub Check: Python==3.12 | django~=4.2.0
- GitHub Check: Python==3.10 | django~=5.0.0
- GitHub Check: Python==3.11 | django~=5.2.0
- GitHub Check: Python==3.13 | django~=5.2.0
- GitHub Check: Python==3.11 | django~=5.1.0
- GitHub Check: Python==3.12 | django~=5.1.0
- GitHub Check: Python==3.10 | django~=5.2.0
- GitHub Check: Python==3.13 | django~=5.1.0
- GitHub Check: Python==3.11 | django~=5.0.0
- GitHub Check: Python==3.10 | django~=4.2.0
- GitHub Check: Python==3.12 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=5.0.0
- GitHub Check: Python==3.11 | django~=4.2.0
- GitHub Check: Python==3.10 | django~=5.1.0
🔇 Additional comments (6)
.github/scripts/ci_failure_bot.py (6)
121-155: LGTM! Well-structured PR diff retrieval.The method appropriately handles missing PR numbers, validates input, truncates large diffs for AI context limits, and includes proper error handling with timeouts.
157-170: LGTM! Correct workflow YAML retrieval.The method correctly fetches the workflow file from the specific commit SHA that triggered the run, ensuring version consistency.
236-249: LGTM! Appropriate fallback response.The fallback provides useful generic guidance and links to documentation when AI analysis is unavailable.
251-279: LGTM! Robust comment deduplication.The method correctly uses an HTML marker and the bot's login to identify and update existing comments, preventing spam on PRs.
338-347: LGTM! Standard entry point with appropriate error handling.The main block follows Python best practices. The broad exception catch flagged by static analysis is acceptable here as a final safety net for unexpected errors.
12-14: The dependencies are already properly pinned in.github/workflows/ci-failure-bot.ymlwith minimum versions:
requests>=2.32.5PyGithub>=2.0.0google-genai>=0.2.0This is the appropriate location for CI script dependencies since the script runs exclusively in the GitHub Actions workflow environment.
|
|
||
| # Initialize Gemini client with new API | ||
| self.client = genai.Client(api_key=self.gemini_api_key) | ||
| self.model_name = os.environ.get("GEMINI_MODEL", "gemini-2.5-flash") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🌐 Web query:
Google Gemini available models gemini-2.5-flash 2025 2026 API
💡 Result:
Short answer: gemini-2.5-flash is a GA model (released June 17, 2025) and is available via Google’s Gemini API and Vertex AI. Google lists its discontinuation/earliest retirement date as June 17, 2026, so you should plan to migrate before then and check the docs for any newer models (e.g., Gemini 3 family) or changes. [1][2][3]
Sources
- Vertex AI / Gemini 2.5 Flash model page (release, GA status, regions, retire date). [1]
- Gemini API changelog (release notes around June–Dec 2025). [2]
- Gemini deprecations table (model release/earliest shutdown dates including gemini-2.5-flash). [3]
Plan migration of Gemini model before June 17, 2026 deprecation date.
The model "gemini-2.5-flash" is valid and currently GA, but has an earliest retirement date of June 17, 2026. Since the code will stop working after that date, consider migrating to a newer model (e.g., Gemini 3 family or later 2.x models) and update the fallback to ensure long-term compatibility.
🤖 Prompt for AI Agents
In @.github/scripts/ci_failure_bot.py at line 56, The default Gemini model
fallback self.model_name = os.environ.get("GEMINI_MODEL", "gemini-2.5-flash")
will retire on 2026-06-17; update the fallback to a currently supported
successor (e.g., a Gemini 3 family model or a later 2.x GA model) and ensure
GEMINI_MODEL remains overridable via env; change the default string used in the
os.environ.get call (symbol: self.model_name) to the new model name, and add a
short code comment referencing the migration/deprecation date so future
maintainers know why the default was updated.
| def analyze_with_gemini(self, build_logs, pr_diff, workflow_yaml): | ||
| """Send context to Gemini for intelligent analysis""" | ||
|
|
||
| # Prepare context for Gemini | ||
| project_name = self.repository_name.split("/")[-1] | ||
| repo_url = f"https://github.com/{self.repository_name}" | ||
| qa_checks_url = f"{repo_url}/blob/master/openwisp-qa-check" | ||
| runtests_url = f"{repo_url}/blob/master/runtests" | ||
|
|
||
| # Build the context string with proper line breaks | ||
| build_logs_json = json.dumps(build_logs, indent=2) | ||
| if pr_diff: | ||
| pr_diff_json = json.dumps(pr_diff, indent=2) | ||
| else: | ||
| pr_diff_json = "No PR associated" | ||
|
|
||
| # Gemini prompt - ignore line length for readability | ||
| context = f""" | ||
| ### ROLE | ||
| You are the "Automated Maintainer Gatekeeper." Your goal is to analyze Pull Request (PR) build failures and provide direct, technically accurate, and no-nonsense feedback to contributors. # noqa: E501 | ||
|
|
||
| ### INPUT CONTEXT PROVIDED | ||
| 1. **Build Output/Logs:** {build_logs_json} | ||
| 2. **YAML Workflow:** {workflow_yaml or "Not available"} | ||
| 3. **PR Diff:** {pr_diff_json} | ||
| 4. **Project Name:** {project_name} # noqa: E501 | ||
| 5. **Repository:** {repo_url} | ||
| 6. **run-qa-checks:** {qa_checks_url} # noqa: E501 | ||
| 7. **runtests:** {runtests_url} # noqa: E501 | ||
|
|
||
| ### TASK | ||
| Analyze the provided context to determine why the build failed. Categorize the failure and respond according to the "Tone Guidelines" below. # noqa: E501 | ||
|
|
||
| ### TONE GUIDELINES | ||
| - **Direct & Honest:** Do not use "fluff" or overly polite corporate language. | ||
| - **Firm Standards:** If a PR is low-effort, spammy, or fails to follow basic instructions, state that clearly. | ||
| - **Action-Oriented:** Provide the exact command or file change needed to fix the error, unless the PR is spammy, in which case we should just declare the PR as potential SPAM and ask maintainers to manually review it. # noqa: E501 | ||
|
|
||
| ### RESPONSE STRUCTURE | ||
| 1. **Status Summary:** A one-sentence blunt assessment of the failure. | ||
| 2. **Technical Diagnosis:** | ||
| - Identify the specific line/test that failed. | ||
| - Explain *why* it failed. | ||
| 3. **Required Action:** Provide a code block or specific steps the contributor must take. | ||
| 4. **Quality Warning (If Applicable):** If the PR appears to be "spam" (e.g., trivial README changes, AI-generated nonsense, or repeated basic errors), include a firm statement that such contributions are a drain on project resources and ping the maintainers asking them for manual review. # noqa: E501 | ||
|
|
||
| ### EXAMPLE RESPONSE STYLE | ||
| "The build failed because you neglected to update the test suite to match your logic changes. This project does not accept functional changes without corresponding test updates. Refer to the log at line 452. Update tests/logic_test.py before re-submitting. We prioritize high-quality, ready-to-merge code; please ensure you run local tests before pushing." # noqa: E501 | ||
|
|
||
| Analyze the failure and provide your response: | ||
| """ | ||
|
|
||
| try: | ||
| # Use new Gemini client API | ||
| from google.genai import types | ||
|
|
||
| response = self.client.models.generate_content( | ||
| model=self.model_name, contents=types.Part.from_text(context) | ||
| ) | ||
| return response.text | ||
| except (ValueError, ConnectionError, Exception) as e: | ||
| print(f"Error calling Gemini API: {e}") | ||
| return self.fallback_response() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# First, let's explore the structure and find relevant files
git ls-files | head -20Repository: openwisp/openwisp-utils
Length of output: 591
🏁 Script executed:
# Look at the CI failure bot file and surrounding context
cd .github/scripts && ls -laRepository: openwisp/openwisp-utils
Length of output: 248
🏁 Script executed:
# Check if there's a README or documentation about the bot
fd -e md -e txt | xargs grep -l "gemini\|ai\|privacy\|security" 2>/dev/null | head -10Repository: openwisp/openwisp-utils
Length of output: 189
🏁 Script executed:
# Search for privacy-related discussions in the codebase
rg -i "privacy|security|data|sanitize" --type py --max-count=20Repository: openwisp/openwisp-utils
Length of output: 8322
🏁 Script executed:
# Look for the run method mentioned in the review (line 301)
rg "def run\|fork.*pr" .github/scripts/ci_failure_bot.py -A 5 -B 2Repository: openwisp/openwisp-utils
Length of output: 49
🏁 Script executed:
# Check for fallback_response implementation
rg "def fallback_response" .github/scripts/ci_failure_bot.py -A 10Repository: openwisp/openwisp-utils
Length of output: 448
🏁 Script executed:
# Look for the run method and fork PR detection
rg "def run|is_fork" .github/scripts/ci_failure_bot.py -B 2 -A 10Repository: openwisp/openwisp-utils
Length of output: 554
🏁 Script executed:
# Check for environment variables and configuration
rg "GITHUB_TOKEN|GEMINI|API|environ" .github/scripts/ci_failure_bot.py -B 2 -A 2Repository: openwisp/openwisp-utils
Length of output: 1754
🏁 Script executed:
# Look at the full file to understand the structure
wc -l .github/scripts/ci_failure_bot.pyRepository: openwisp/openwisp-utils
Length of output: 104
🏁 Script executed:
# Check the workflow file to understand how it's triggered
cat .github/workflows/ci-failure-bot.ymlRepository: openwisp/openwisp-utils
Length of output: 1652
🏁 Script executed:
# Look for any issues, discussions, or PR comments about privacy/security
rg -i "privacy|security|gemini|data sharing|external" . --type md --type rst --type txtRepository: openwisp/openwisp-utils
Length of output: 1932
🏁 Script executed:
# Check if there's any log sanitization or filtering
rg "filter|sanitize|mask|redact|secret|token" .github/scripts/ci_failure_bot.py -B 2 -A 2Repository: openwisp/openwisp-utils
Length of output: 1192
🏁 Script executed:
# Look at the beginning of the file to understand initialization and API key handling
head -100 .github/scripts/ci_failure_bot.pyRepository: openwisp/openwisp-utils
Length of output: 3938
🏁 Script executed:
# Get the full run method to see fork PR detection
sed -n '265,330p' .github/scripts/ci_failure_bot.pyRepository: openwisp/openwisp-utils
Length of output: 2765
🏁 Script executed:
# Look for fork-related code
rg "fork\|is_fork\|event.workflow_run" .github/scripts/ci_failure_bot.py -B 2 -A 5Repository: openwisp/openwisp-utils
Length of output: 49
🏁 Script executed:
# Check the get_pr_diff method to see if there's any fork handling
rg "def get_pr_diff" .github/scripts/ci_failure_bot.py -A 30Repository: openwisp/openwisp-utils
Length of output: 1198
🏁 Script executed:
# Look at the documentation files mentioned
cat docs/user/ci-failure-bot.rst | head -80Repository: openwisp/openwisp-utils
Length of output: 2496
🏁 Script executed:
# Check if there's any rate limiting or cost monitoring
rg "rate|limit|cost|quota" .github/scripts/ci_failure_bot.py -B 2 -A 2Repository: openwisp/openwisp-utils
Length of output: 1430
🏁 Script executed:
# Get the context where Gemini is called to see the full data being sent
sed -n '172,240p' .github/scripts/ci_failure_bot.pyRepository: openwisp/openwisp-utils
Length of output: 3596
Fork PR security check is already implemented; privacy implications are documented.
Fork PRs are explicitly skipped (lines 287-293) with a comment confirming this is for security to avoid sending external code to AI. Privacy considerations are also already documented in docs/user/ci-failure-bot.rst, which warns that "PR diffs and build logs are sent to Google's Gemini AI service" and recommends organizations with sensitive codebases be aware of this.
Remove unnecessary noqa comments from the Gemini prompt.
Lines 189–222 include # noqa: E501 comments within the prompt string sent to Gemini. These should be removed since they're runtime data, not code being linted, and serve no purpose in the API request.
Fix hard-coded master branch reference and broad exception catch.
- Lines 176–177 assume the repository uses a
masterbranch for the URLs, which may not match if the default branch differs. - Line 232 catches the broad
Exceptionclass instead of specific exception types, which bypasses linter checks.
🧰 Tools
🪛 Ruff (0.14.10)
231-231: Consider moving this statement to an else block
(TRY300)
232-232: Do not catch blind exception: Exception
(BLE001)
| context = f""" | ||
| ### ROLE | ||
| You are the "Automated Maintainer Gatekeeper." Your goal is to analyze Pull Request (PR) build failures and provide direct, technically accurate, and no-nonsense feedback to contributors. # noqa: E501 | ||
|
|
||
| ### INPUT CONTEXT PROVIDED | ||
| 1. **Build Output/Logs:** {build_logs_json} | ||
| 2. **YAML Workflow:** {workflow_yaml or "Not available"} | ||
| 3. **PR Diff:** {pr_diff_json} | ||
| 4. **Project Name:** {project_name} # noqa: E501 | ||
| 5. **Repository:** {repo_url} | ||
| 6. **run-qa-checks:** {qa_checks_url} # noqa: E501 | ||
| 7. **runtests:** {runtests_url} # noqa: E501 | ||
|
|
||
| ### TASK | ||
| Analyze the provided context to determine why the build failed. Categorize the failure and respond according to the "Tone Guidelines" below. # noqa: E501 | ||
|
|
||
| ### TONE GUIDELINES | ||
| - **Direct & Honest:** Do not use "fluff" or overly polite corporate language. | ||
| - **Firm Standards:** If a PR is low-effort, spammy, or fails to follow basic instructions, state that clearly. | ||
| - **Action-Oriented:** Provide the exact command or file change needed to fix the error, unless the PR is spammy, in which case we should just declare the PR as potential SPAM and ask maintainers to manually review it. # noqa: E501 | ||
|
|
||
| ### RESPONSE STRUCTURE | ||
| 1. **Status Summary:** A one-sentence blunt assessment of the failure. | ||
| 2. **Technical Diagnosis:** | ||
| - Identify the specific line/test that failed. | ||
| - Explain *why* it failed. | ||
| 3. **Required Action:** Provide a code block or specific steps the contributor must take. | ||
| 4. **Quality Warning (If Applicable):** If the PR appears to be "spam" (e.g., trivial README changes, AI-generated nonsense, or repeated basic errors), include a firm statement that such contributions are a drain on project resources and ping the maintainers asking them for manual review. # noqa: E501 | ||
|
|
||
| ### EXAMPLE RESPONSE STYLE | ||
| "The build failed because you neglected to update the test suite to match your logic changes. This project does not accept functional changes without corresponding test updates. Refer to the log at line 452. Update tests/logic_test.py before re-submitting. We prioritize high-quality, ready-to-merge code; please ensure you run local tests before pushing." # noqa: E501 | ||
|
|
||
| Analyze the failure and provide your response: | ||
| """ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove noqa comments from the prompt string.
The prompt contains # noqa: E501 comments (lines 191, 197, 199, 200, 203, 208, 216, 219) intended for linters, but these will be included in the actual text sent to Gemini. These comments serve no purpose in the runtime prompt and should be removed.
🧹 Proposed fix to remove noqa comments
context = f"""
### ROLE
-You are the "Automated Maintainer Gatekeeper." Your goal is to analyze Pull Request (PR) build failures and provide direct, technically accurate, and no-nonsense feedback to contributors. # noqa: E501
+You are the "Automated Maintainer Gatekeeper." Your goal is to analyze Pull Request (PR) build failures and provide direct, technically accurate, and no-nonsense feedback to contributors.
### INPUT CONTEXT PROVIDED
1. **Build Output/Logs:** {build_logs_json}
2. **YAML Workflow:** {workflow_yaml or "Not available"}
3. **PR Diff:** {pr_diff_json}
-4. **Project Name:** {project_name} # noqa: E501
+4. **Project Name:** {project_name}
5. **Repository:** {repo_url}
-6. **run-qa-checks:** {qa_checks_url} # noqa: E501
-7. **runtests:** {runtests_url} # noqa: E501
+6. **run-qa-checks:** {qa_checks_url}
+7. **runtests:** {runtests_url}
### TASK
-Analyze the provided context to determine why the build failed. Categorize the failure and respond according to the "Tone Guidelines" below. # noqa: E501
+Analyze the provided context to determine why the build failed. Categorize the failure and respond according to the "Tone Guidelines" below.
### TONE GUIDELINES
- **Direct & Honest:** Do not use "fluff" or overly polite corporate language.
- **Firm Standards:** If a PR is low-effort, spammy, or fails to follow basic instructions, state that clearly.
-- **Action-Oriented:** Provide the exact command or file change needed to fix the error, unless the PR is spammy, in which case we should just declare the PR as potential SPAM and ask maintainers to manually review it. # noqa: E501
+- **Action-Oriented:** Provide the exact command or file change needed to fix the error, unless the PR is spammy, in which case we should just declare the PR as potential SPAM and ask maintainers to manually review it.
### RESPONSE STRUCTURE
1. **Status Summary:** A one-sentence blunt assessment of the failure.
2. **Technical Diagnosis:**
- Identify the specific line/test that failed.
- Explain *why* it failed.
3. **Required Action:** Provide a code block or specific steps the contributor must take.
-4. **Quality Warning (If Applicable):** If the PR appears to be "spam" (e.g., trivial README changes, AI-generated nonsense, or repeated basic errors), include a firm statement that such contributions are a drain on project resources and ping the maintainers asking them for manual review. # noqa: E501
+4. **Quality Warning (If Applicable):** If the PR appears to be "spam" (e.g., trivial README changes, AI-generated nonsense, or repeated basic errors), include a firm statement that such contributions are a drain on project resources and ping the maintainers asking them for manual review.
### EXAMPLE RESPONSE STYLE
-"The build failed because you neglected to update the test suite to match your logic changes. This project does not accept functional changes without corresponding test updates. Refer to the log at line 452. Update tests/logic_test.py before re-submitting. We prioritize high-quality, ready-to-merge code; please ensure you run local tests before pushing." # noqa: E501
+"The build failed because you neglected to update the test suite to match your logic changes. This project does not accept functional changes without corresponding test updates. Refer to the log at line 452. Update tests/logic_test.py before re-submitting. We prioritize high-quality, ready-to-merge code; please ensure you run local tests before pushing."
Analyze the failure and provide your response:
"""🤖 Prompt for AI Agents
In @.github/scripts/ci_failure_bot.py around lines 189 - 222, The prompt string
assigned to the variable context in .github/scripts/ci_failure_bot.py includes
inline linter comments "# noqa: E501" which will be sent to the LLM; remove all
occurrences of these comments from the f-string so the runtime prompt is clean.
Edit the context f-string (the multi-line prompt) to delete every "# noqa: E501"
token (e.g., after the ROLE line and any lines referencing project_name,
repo_url, qa_checks_url, runtests_url, and RESPONSE STRUCTURE) so the content
sent to Gemini contains only the intended prompt text.
| # Skip fork PRs for security (avoid sending external code to AI) | ||
| if self.pr_number and self.pr_number.strip(): | ||
| try: | ||
| pr_num = int(self.pr_number) | ||
| pr = self.repo.get_pull(pr_num) | ||
| if pr.head.repo.full_name != self.repository_name: | ||
| print(f"Skipping fork PR from {pr.head.repo.full_name}") | ||
| return | ||
| except (GithubException, ValueError) as e: | ||
| print(f"Warning: Could not check fork status: {e}") | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Handle deleted fork repositories gracefully.
If a PR's head repository has been deleted (common with forks), pr.head.repo will be None, causing pr.head.repo.full_name on line 301 to raise an AttributeError. While the exception is caught generically, it's better to check explicitly.
🛡️ Proposed fix to check for deleted repos
if self.pr_number and self.pr_number.strip():
try:
pr_num = int(self.pr_number)
pr = self.repo.get_pull(pr_num)
- if pr.head.repo.full_name != self.repository_name:
- print(f"Skipping fork PR from {pr.head.repo.full_name}")
+ if pr.head.repo and pr.head.repo.full_name != self.repository_name:
+ print(f"Skipping fork PR from {pr.head.repo.full_name}")
+ return
+ elif not pr.head.repo:
+ print("Skipping PR with deleted head repository")
return
except (GithubException, ValueError) as e:
print(f"Warning: Could not check fork status: {e}")Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In @.github/scripts/ci_failure_bot.py around lines 296 - 306, The current fork
check accesses pr.head.repo.full_name which will raise when pr.head.repo is None
for deleted fork repositories; update the block that handles self.pr_number/pr
to explicitly check if pr.head.repo is None before accessing full_name and treat
that case as a deleted fork (print a clear message and return/skip), otherwise
compare pr.head.repo.full_name to self.repository_name as before; keep the
existing except (GithubException, ValueError) handling for API errors but avoid
relying on it for the deleted-repo case by performing the explicit None check on
pr.head.repo.
|
Hi @nemesifier , thanks for the guidance! I updated environment reinstalled the QA tools with pip install -U --force-reinstall -e .[qa], ran openwisp-qa-format, and verified everything with ./run-qa-checks. All CI checks are now passing . I've also addressed the critical CodeRabbit review points, while keeping the remaining suggestions minimal as intended. Please let me know if you'd like any further changes. Thanks. |
Implements automated bot responses for common CI failures:
Checklist
Reference to Existing Issue
Addresses #524
Description of Changes
Adds GitHub Actions workflow and Python script for automated CI failure responses.
Screenshot
Test repository showing bot functionality: https://github.com/Srinath0916/ci-bot-test