fix: run neovim-upgrade in dev shell on CI and add daily Renovate workflow - #770
Conversation
…kflow - neovim-upgrade now delegates to neovim-upgrade-dev in CI/Docker, running nvim via nix develop where it is available (fixes Error 127) - Add self-hosted Renovate workflow running daily at 3am UTC
|
You do not have enough credits to review this pull request. Please purchase more credits to continue. |
Summary of ChangesHello @shunkakinoki, 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 addresses a critical failure in the daily Neovim upgrade workflow on CI and introduces a new automated process for dependency management. The changes ensure that Neovim plugin updates proceed smoothly in automated environments and establish a regular schedule for checking and updating project dependencies. Highlights
Changelog
Ignored Files
Activity
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. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Mesa DescriptionTL;DRFixed the daily failing Neovim plugin upgrade CI job by adjusting how What changed?
Description generated by Mesa. Update settings |
📝 WalkthroughWalkthroughAdds a Renovate GitHub Actions workflow and tweaks workflows and Makefile to run Neovim plugin updates inside the Nix dev shell in CI/Docker; also reformats a nix expression. Changes include workflow triggers, a new Makefile target, and conditional invocation of that target. Changes
Sequence Diagram(s)sequenceDiagram
participant Scheduler as "GitHub Scheduler / Manual"
participant GHActions as "GitHub Actions"
participant Checkout as "actions/checkout"
participant Renovate as "renovatebot/github-action"
participant Repo as "Repository (renovate.json)"
participant AllsGreen as "Alls Green Action"
Scheduler->>GHActions: trigger (cron or workflow_dispatch)
GHActions->>Checkout: checkout repo
GHActions->>Renovate: run renovate action (uses `renovate.json`, PAT_TOKEN)
Renovate->>Repo: read `renovate.json` config
Renovate-->>GHActions: produce results / PRs
GHActions->>AllsGreen: after `needs: renovate`, report status
AllsGreen-->>GHActions: status report
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Code Review
This pull request aims to fix a failing Neovim upgrade workflow in CI by running the upgrade command within a Nix development shell, introducing a new neovim-upgrade-dev Makefile target and modifying the existing neovim-upgrade target. However, the current implementation introduces a critical logic flaw in the Makefile that leads to infinite recursion and Denial of Service in CI/Docker environments, as the neovim-upgrade target recursively delegates to itself via neovim-upgrade-dev without a termination condition. Furthermore, there are potential command injection vulnerabilities due to unquoted variable expansions in shell commands. Addressing these issues is crucial for the stability and security of the CI pipeline.
| @if [ "$$CI" = "true" ] || [ "$$IN_DOCKER" = "true" ]; then \ | ||
| $(MAKE) neovim-upgrade-dev; \ | ||
| else \ | ||
| nvim --headless +"lua vim.pack.update()" +qa; \ | ||
| fi | ||
| @echo "✅ Neovim plugins updated" | ||
|
|
||
| .PHONY: neovim-upgrade-dev | ||
| neovim-upgrade-dev: ## Update Neovim plugins inside the Nix dev shell (mirrors CI). | ||
| @echo "📦 Updating neovim plugins inside the Nix dev shell..." | ||
| @DEVENV_ROOT=$(CURDIR) $(NIX_ALLOW_UNFREE) $(NIX_EXEC) develop $(NIX_FLAGS) .# --command $(MAKE) neovim-upgrade |
There was a problem hiding this comment.
The current implementation of neovim-upgrade and neovim-upgrade-dev creates an infinite recursion loop when running in CI or Docker, leading to a Denial of Service (DoS) of the CI pipeline. This occurs because neovim-upgrade (line 584) calls neovim-upgrade-dev if CI or IN_DOCKER is true. neovim-upgrade-dev (line 594) then executes make neovim-upgrade inside a nix develop shell. Since environment variables like CI are typically preserved, the recursive call to make neovim-upgrade will again see CI=true and call neovim-upgrade-dev again, causing an infinite loop. To resolve this, the nvim command should be executed directly from within the nix develop command, instead of re-invoking make neovim-upgrade.
| .PHONY: neovim-upgrade-dev | ||
| neovim-upgrade-dev: ## Update Neovim plugins inside the Nix dev shell (mirrors CI). | ||
| @echo "📦 Updating neovim plugins inside the Nix dev shell..." | ||
| @DEVENV_ROOT=$(CURDIR) $(NIX_ALLOW_UNFREE) $(NIX_EXEC) develop $(NIX_FLAGS) .# --command $(MAKE) neovim-upgrade |
There was a problem hiding this comment.
The variable $(CURDIR) is used without quotes when setting the DEVENV_ROOT environment variable. If the repository is cloned into a directory path containing spaces or shell metacharacters (e.g., ;, &, |), this can lead to command injection or unexpected behavior as the shell interprets the unquoted path.
Additionally, the environment variables CI and IN_DOCKER are used in a shell if condition. While they are wrapped in double quotes, a value containing a double quote could potentially break out of the condition and execute arbitrary commands.
@DEVENV_ROOT="$(CURDIR)" $(NIX_ALLOW_UNFREE) $(NIX_EXEC) develop $(NIX_FLAGS) .# --command nvim --headless +"lua vim.pack.update()" +qa
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@Makefile`:
- Around line 586-597: The neovim-upgrade target delegates to neovim-upgrade-dev
in CI, but neovim-upgrade-dev currently re-enters the dev shell and runs "make
neovim-upgrade", causing infinite recursion; modify the neovim-upgrade-dev
recipe so it invokes the nvim update command directly inside the Nix dev shell
(i.e., use $(NIX_EXEC) develop $(NIX_FLAGS) .# --command to run the same nvim
--headless +"lua vim.pack.update()" +qa command) instead of calling make
neovim-upgrade.
🧹 Nitpick comments (1)
.github/workflows/renovate.yml (1)
1-8: Add a top-levelpermissionsblock to restrict token scope.The workflow has no
permissionsdeclaration. Since Renovate only needs to read repo contents and create PRs via the PAT, the defaultGITHUB_TOKENpermissions should be restricted. This follows the guideline to "limit permissions to minimum required."Proposed fix
name: Renovate on: schedule: - cron: '0 3 * * *' workflow_dispatch: +permissions: {} concurrency:As per coding guidelines: "Use GITHUB_TOKEN when possible, secure sensitive data in repository secrets, and limit permissions to minimum required in GitHub Actions workflows."
There was a problem hiding this comment.
Pull request overview
Fixes CI failures in the Neovim plugin upgrade automation by attempting to run the upgrade inside the Nix dev shell in CI/Docker contexts, and introduces a scheduled GitHub Actions workflow to run Renovate daily for dependency updates.
Changes:
- Update
make neovim-upgradeto delegate to a dev-shell-based upgrade path on CI/Docker. - Add a new scheduled (
cron) Renovate GitHub Actions workflow with an “alls-green” aggregator job.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| Makefile | Adds CI/Docker-aware delegation for Neovim plugin upgrades and introduces a dev-shell upgrade target. |
| .github/workflows/renovate.yml | Adds a daily scheduled Renovate workflow plus a summary check job. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @if [ "$$CI" = "true" ] || [ "$$IN_DOCKER" = "true" ]; then \ | ||
| $(MAKE) neovim-upgrade-dev; \ | ||
| else \ |
There was a problem hiding this comment.
This introduces infinite recursion in CI/Docker: neovim-upgrade delegates to neovim-upgrade-dev, which runs make neovim-upgrade inside nix develop; since CI is still true in that environment, it will delegate again and loop until failure. Introduce a non-recursive inner target (e.g., one that runs the nvim --headless ... command unconditionally) and have the wrapper target delegate to that, or explicitly override/unset CI/IN_DOCKER for the inner invocation.
| neovim-upgrade-dev: ## Update Neovim plugins inside the Nix dev shell (mirrors CI). | ||
| @echo "📦 Updating neovim plugins inside the Nix dev shell..." | ||
| @DEVENV_ROOT=$(CURDIR) $(NIX_ALLOW_UNFREE) $(NIX_EXEC) develop $(NIX_FLAGS) .# --command $(MAKE) neovim-upgrade |
There was a problem hiding this comment.
neovim-upgrade-dev runs nix develop ... --command $(MAKE) neovim-upgrade, but neovim-upgrade is a wrapper that conditionally delegates back to neovim-upgrade-dev when CI/IN_DOCKER is set, creating a recursion loop. Consider running nvim --headless ... directly as the --command, or call a dedicated non-wrapper target (e.g., neovim-upgrade-inner) from both places.
| concurrency: | ||
| group: ${{ github.workflow }}-${{ github.event_name }}-${{ github.head_ref || github.sha }} | ||
| cancel-in-progress: true |
There was a problem hiding this comment.
The concurrency group includes ${{ github.sha }}, which changes on every scheduled run; this prevents cancel-in-progress: true from cancelling a previous still-running scheduled Renovate job (each run will end up in a different group). For schedule-based workflows, consider using a stable key like ${{ github.workflow }}-${{ github.event_name }} (optionally plus ${{ github.ref }}) so overlapping runs get cancelled as intended.
neovim-upgrade-dev now runs nvim directly via nix develop instead of calling make neovim-upgrade, which would re-detect CI and loop.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.github/workflows/upgrade.yml:
- Line 30: The workflow conditional is using the wrong context key: replace the
condition "if: github.action != 'pull_request'" with a check against the event
name (e.g., use github.event_name != 'pull_request') so the step actually skips
on pull_request events; update the conditional expression where it appears in
the upgrade.yml workflow to reference github.event_name instead of
github.action.
| GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
| SKIP_HOME_MANAGER_SWITCH: "true" | ||
| - name: Create Pull Request | ||
| if: github.action != 'pull_request' |
There was a problem hiding this comment.
Bug: github.action should be github.event_name.
github.action is the unique identifier of the step action (or the action's repo slug), not the event trigger name. This condition will almost certainly always evaluate to true, so PR creation will not be skipped on pull_request events as intended.
Proposed fix
- if: github.action != 'pull_request'
+ if: github.event_name != 'pull_request'📝 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.
| if: github.action != 'pull_request' | |
| if: github.event_name != 'pull_request' |
🤖 Prompt for AI Agents
In @.github/workflows/upgrade.yml at line 30, The workflow conditional is using
the wrong context key: replace the condition "if: github.action !=
'pull_request'" with a check against the event name (e.g., use github.event_name
!= 'pull_request') so the step actually skips on pull_request events; update the
conditional expression where it appears in the upgrade.yml workflow to reference
github.event_name instead of github.action.
This reverts commit 24a8d34.
upgrade-dev wraps make upgrade inside nix develop, so all devshell tools (including nvim) are available. No CI guards or recursion needed.
Changes
neovim-upgradenow auto-delegates toneovim-upgrade-devin CI/Docker, runningnvimvianix develop(fixes Error 127: command not found)renovate.yml) running daily at 3am UTC for consistent dependency update checksTesting
workflow_dispatchGenerated with Claude Code by claude-opus-4-6
Summary by cubic
Fixes the failing upgrade job by running the whole upgrade inside the Nix dev shell via a new upgrade-dev target, and adds a daily Renovate workflow to keep dependencies up to date. Also updates the upgrade schedule and simplifies Ghostty config.
Bug Fixes and Refactors
New Features
Written for commit 5666f8f. Summary will update on new commits.