fix(automation): validate action ref existence and add major bump warnings#663
fix(automation): validate action ref existence and add major bump warnings#663
Conversation
…nings Co-authored-by: abhimehro <84992105+abhimehro@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
Merging to
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| mock_gh_text.side_effect = ["", "v1.2.3"] | ||
| mock_gh_json.return_value = "v2.0.0" | ||
|
|
||
| assert latest_tag_for_action("actions/checkout") == "v2.0.0" |
Check notice
Code scanning / Bandit
Use of assert detected. The enclosed code will be removed when compiling to optimised byte code. Note test
| mock_gh_text.side_effect = ["", ""] | ||
| mock_gh_json.return_value = None | ||
|
|
||
| assert latest_tag_for_action("actions/checkout") == "" |
Check notice
Code scanning / Bandit
Use of assert detected. The enclosed code will be removed when compiling to optimised byte code. Note test
|
|
||
| def test_target_ref_valid_and_invalid(): | ||
| # Valid upgrades | ||
| assert target_ref("v4", "v5.0.0") == "v5" |
Check notice
Code scanning / Bandit
Use of assert detected. The enclosed code will be removed when compiling to optimised byte code. Note test
| def test_target_ref_valid_and_invalid(): | ||
| # Valid upgrades | ||
| assert target_ref("v4", "v5.0.0") == "v5" | ||
| assert target_ref("v4.2.2", "v4.3.0") == "v4.3.0" |
Check notice
Code scanning / Bandit
Use of assert detected. The enclosed code will be removed when compiling to optimised byte code. Note test
| assert target_ref("v4", "v5.0.0") == "v5" | ||
| assert target_ref("v4.2.2", "v4.3.0") == "v4.3.0" | ||
| # When current is 'v4' and latest is 'v4.3.0', target_ref returns 'v4' which triggers a skip later | ||
| assert target_ref("v4", "v4.3.0") == "v4" |
Check notice
Code scanning / Bandit
Use of assert detected. The enclosed code will be removed when compiling to optimised byte code. Note test
| mock_target_ref.return_value = "v5" | ||
| mock_ref_exists.return_value = False | ||
| plans_missing = workflow_file_plans() | ||
| assert len(plans_missing) == 0 # Should be skipped because ref doesn't exist |
Check notice
Code scanning / Bandit
Use of assert detected. The enclosed code will be removed when compiling to optimised byte code. Note test
| mock_ref_exists.return_value = True | ||
| plans_exist = workflow_file_plans() | ||
|
|
||
| assert len(plans_exist) == 1 |
Check notice
Code scanning / Bandit
Use of assert detected. The enclosed code will be removed when compiling to optimised byte code. Note test
| plans_exist = workflow_file_plans() | ||
|
|
||
| assert len(plans_exist) == 1 | ||
| assert plans_exist[0]["replacements"][0]["target"] == "v4" |
Check notice
Code scanning / Bandit
Use of assert detected. The enclosed code will be removed when compiling to optimised byte code. Note test
|
|
||
| assert len(plans_exist) == 1 | ||
| assert plans_exist[0]["replacements"][0]["target"] == "v4" | ||
| assert plans_exist[0]["replacements"][0]["is_major_bump"] is True |
Check notice
Code scanning / Bandit
Use of assert detected. The enclosed code will be removed when compiling to optimised byte code. Note test
| assert fix_env.escape_val("value") == "value" | ||
| assert fix_env.escape_val('val"ue') == 'val\\"ue' | ||
| assert fix_env.escape_val('val\\ue') == 'val\\\\ue' | ||
| assert fix_env.escape_val("val\\ue") == "val\\\\ue" |
Check notice
Code scanning / Bandit
Use of assert detected. The enclosed code will be removed when compiling to optimised byte code. Note test
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the repository automation workflow updater by addressing critical issues related to GitHub Action version management. It introduces robust validation to ensure that proposed action references exist before applying updates and provides clear warnings for major version bumps, improving the reliability and safety of automated workflow updates. Highlights
Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![codescene-delta-analysis[bot]](https://avatars.githubusercontent.com/in/53921?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request significantly improves the repository automation workflow updater by adding validation for action references and introducing warnings for major version bumps. The logic is sound, and the inclusion of unit tests is a great addition. I've identified two edge cases in the jq queries for fetching latest tags that could lead to script failures when a repository has no stable releases or no tags. My review includes suggestions to make these queries more robust to prevent such crashes.
… code health Co-authored-by: abhimehro <84992105+abhimehro@users.noreply.github.com>
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Co-authored-by: abhimehro <84992105+abhimehro@users.noreply.github.com>
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
…odeScene errors Co-authored-by: abhimehro <84992105+abhimehro@users.noreply.github.com>
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Gates Failed
(1 hotspot with Complex Method)
(1 file with Low Cohesion)
(2 files with Lines of Code in a Single File, Large Method, Complex Method)
Gates Passed
See analysis details in CodeScene
Reason for failure
| Prevent hotspot decline | Violations | Code Health Impact | |
|---|---|---|---|
| main.py | 1 rule in this hotspot | 1.61 → 1.61 | Suppress |
| Enforce critical code health rules | Violations | Code Health Impact | |
|---|---|---|---|
| repository_automation_tasks.py | 1 critical rule | 8.22 → 7.42 | Suppress |
| Enforce advisory code health rules | Violations | Code Health Impact | |
|---|---|---|---|
| repository_automation_tasks.py | 2 advisory rules | 8.22 → 7.42 | Suppress |
| main.py | 1 advisory rule | 1.61 → 1.61 | Suppress |
Quality Gate Profile: Pay Down Tech Debt
Install CodeScene MCP: safeguard and uplift AI-generated code. Catch issues early with our IDE extension and CLI tool.
1 participant
This PR fixes a bug in the repository automation workflow updater. The updater was previously failing to check if the proposed latest major version tag (like
v6) actually existed in the repository, resulting in broken updates likeactions/checkout@v6. It also failed to warn users about major version bumps, silently hiding breaking changes.Changes:
ref_exists: Inrepository_automation_common.py, added a newref_existsmethod that usesgh apito check if a proposed target tag exists by queryinggit/refs/tags/{ref}first and then falling back togit/refs/heads/{ref}(since many actions use branches for major updates).latest_tag_for_actionto prioritize proper, non-prerelease tags by inspecting the.prereleaseproperty on releases, filtering out API glitches (such as prereleases marked latest).workflow_file_plansinrepository_automation_tasks.pyto evaluate the proposed refs withref_existsbefore appending them to the replacement plan. If it doesn't exist, the update is skipped.is_major_bumpper update action. Added logic inrun_workflow_updaterto append a### Compatibility review requiredsection to the PR body with bullet points listing specific actions needing review before merging major upgrades.tests/test_automation/test_workflow_updater.pymimicking GitHub API calls and checking proper skip behaviors based on internal configurations.PR created automatically by Jules for task 15796309238167997888 started by @abhimehro