Skip to content

checkPlugin: update all plugin dependencies during autofix#7404

Merged
JohnMcLear merged 1 commit into
ether:developfrom
JohnMcLear:fix/checkplugin-update-deps
Mar 31, 2026
Merged

checkPlugin: update all plugin dependencies during autofix#7404
JohnMcLear merged 1 commit into
ether:developfrom
JohnMcLear:fix/checkplugin-update-deps

Conversation

@JohnMcLear

Copy link
Copy Markdown
Member

Summary

  • Add pnpm update step to checkPlugin.ts when running in autofix mode
  • Bumps all dependencies (not just devDependencies) to latest compatible versions
  • Reduces dependabot PR noise across plugin repos
  • Runs before pnpm install and linting

Test plan

  • Run pnpm run checkPlugin ep_some_plugin autofix and verify dependencies are updated
  • Verify lockfile is regenerated with updated versions

🤖 Generated with Claude Code

Run pnpm update on plugins during autofix to bump all dependencies
to their latest compatible versions, reducing dependabot noise and
keeping plugins current.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@qodo-free-for-open-source-projects

Copy link
Copy Markdown

Review Summary by Qodo

Add pnpm update step to checkPlugin autofix mode

✨ Enhancement

Grey Divider

Walkthroughs

Description
• Add pnpm update step during autofix mode
• Updates all plugin dependencies to latest compatible versions
• Reduces dependabot PR noise across plugin repositories
• Executes before pnpm install and linting steps
Diagram
flowchart LR
  autoFix["autoFix mode enabled"]
  update["pnpm update<br/>dependencies"]
  install["pnpm install"]
  lint["ESLint & linting"]
  autoFix -- "if true" --> update
  update --> install
  install --> lint
Loading

Grey Divider

File Changes

1. bin/plugins/checkPlugin.ts ✨ Enhancement +6/-0

Add dependency update step in autofix mode

• Added conditional pnpm update execution when autofix mode is enabled
• Logs informational message before updating dependencies
• Positioned before pnpm install to ensure latest compatible versions
• Uses execSync with inherited stdio for transparent output

bin/plugins/checkPlugin.ts


Grey Divider

Qodo Logo

@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Mar 31, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider


Action required

1. Update failure aborts autofix 🐞 Bug ☼ Reliability
Description
In autofix mode, checkPlugin.ts runs pnpm update via execSync() without a try/catch, so any
non-zero exit (registry/network/resolution failure) terminates the script before
install/lint/diff/commit steps can run.
Code

bin/plugins/checkPlugin.ts[R406-410]

+  // Update all dependencies to their latest compatible versions.
+  if (autoFix) {
+    logger.info('Updating dependencies...');
+    execSync('pnpm update', {cwd: `${pluginPath}/`, stdio: 'inherit'});
+  }
Evidence
The new pnpm update call is unguarded, while other potentially-failing steps (like linting) are
explicitly wrapped in try/catch to allow the script to continue and provide guidance.

bin/plugins/checkPlugin.ts[406-410]
bin/plugins/checkPlugin.ts[425-432]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`pnpm update` is executed with `execSync()` in autofix mode without error handling; if it fails, the script exits early and skips later autofix steps (install, lint, showing diff, and optional commit/push).

### Issue Context
There is already precedent in this script for tolerating failures and continuing (the lint section wraps `execSync()` in a try/catch).

### Fix Focus Areas
- bin/plugins/checkPlugin.ts[406-410]
- bin/plugins/checkPlugin.ts[425-432]

### Suggested change
Wrap the `pnpm update` `execSync()` call in a try/catch. On failure, log a warning/error and continue to the existing `pnpm install` + linting flow (or explicitly rethrow with a clearer message if you intend update failures to be fatal).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

2. Extra pnpm invocation added 🐞 Bug ➹ Performance
Description
In autofix mode the script now invokes pnpm multiple times (pnpm update and then pnpm install,
plus an additional pnpm install earlier when the lockfile is missing), increasing runtime and the
chance of transient failures.
Code

bin/plugins/checkPlugin.ts[R406-414]

+  // Update all dependencies to their latest compatible versions.
+  if (autoFix) {
+    logger.info('Updating dependencies...');
+    execSync('pnpm update', {cwd: `${pluginPath}/`, stdio: 'inherit'});
+  }
+
  // Install dependencies so we can run ESLint. This should also create or update package-lock.json
  // if autoFix is enabled.
  const npmInstall = `pnpm install`;
Evidence
The new code adds a pnpm update call and the script still runs pnpm install afterwards;
additionally, when pnpm-lock.yaml is missing and autofix is enabled, the script already runs `pnpm
install` earlier in the same run.

bin/plugins/checkPlugin.ts[406-415]
bin/plugins/checkPlugin.ts[269-285]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
Autofix mode now runs multiple pnpm commands in one execution path (`pnpm update` + `pnpm install`, and in some cases an extra earlier `pnpm install`). This increases total execution time and adds additional points of failure.

### Issue Context
The script already has a dedicated lockfile-missing autofix branch that calls `pnpm install`, and later it unconditionally calls `pnpm install` for linting.

### Fix Focus Areas
- bin/plugins/checkPlugin.ts[269-285]
- bin/plugins/checkPlugin.ts[406-415]

### Suggested change
Ensure the script performs dependency operations at most once per run (or as few times as necessary). For example, remove the earlier `pnpm install` in the lockfile-missing autofix branch since a `pnpm install` is performed later anyway, and consider consolidating the autofix `update` + `install` flow into a single, clearly-defined step.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

@JohnMcLear JohnMcLear merged commit 09df1ce into ether:develop Mar 31, 2026
24 checks passed
Comment on lines +406 to +410
// Update all dependencies to their latest compatible versions.
if (autoFix) {
logger.info('Updating dependencies...');
execSync('pnpm update', {cwd: `${pluginPath}/`, stdio: 'inherit'});
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

1. Update failure aborts autofix 🐞 Bug ☼ Reliability

In autofix mode, checkPlugin.ts runs pnpm update via execSync() without a try/catch, so any
non-zero exit (registry/network/resolution failure) terminates the script before
install/lint/diff/commit steps can run.
Agent Prompt
### Issue description
`pnpm update` is executed with `execSync()` in autofix mode without error handling; if it fails, the script exits early and skips later autofix steps (install, lint, showing diff, and optional commit/push).

### Issue Context
There is already precedent in this script for tolerating failures and continuing (the lint section wraps `execSync()` in a try/catch).

### Fix Focus Areas
- bin/plugins/checkPlugin.ts[406-410]
- bin/plugins/checkPlugin.ts[425-432]

### Suggested change
Wrap the `pnpm update` `execSync()` call in a try/catch. On failure, log a warning/error and continue to the existing `pnpm install` + linting flow (or explicitly rethrow with a clearer message if you intend update failures to be fatal).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant