fix(cli): prevent concurrent extension install races - #29087
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
|
📊 PR Size: size/L
|
gemini-cli
Bot
added
the
area/extensions
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 addresses a race condition in the Gemini CLI where concurrent processes could simultaneously modify the same extension directory. By implementing an exclusive locking mechanism, the changes ensure that only one process can perform installation or update operations on a specific extension at any given time. The solution includes robust cleanup of lock files and handles edge cases like renames and failed operations to maintain system stability. Highlights
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 the 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 counterproductive. 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
|
![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 introduces concurrent installation locking for extensions using the proper-lockfile library to prevent race conditions during installations, updates, or renames. It also adds comprehensive tests to verify this behavior. A critical issue was identified where locking a non-existent extension directory during a new installation will throw an ENOENT error; it is recommended to create and lock a stable placeholder file inside the locks directory instead.
| for (const extensionName of uniqueNames) { | ||
| const extensionPath = new ExtensionStorage( | ||
| extensionName, | ||
| ).getExtensionDir(); | ||
| releases.push( | ||
| await lock(extensionPath, { | ||
| realpath: false, | ||
| // Keep lock directories outside the extensions directory because the | ||
| // loader treats every entry there as a potential extension. | ||
| lockfilePath: path.join(locksDir, `${extensionName}.lock`), | ||
| }), | ||
| ); | ||
| } |
There was a problem hiding this comment.
The proper-lockfile library requires the target path to exist before attempting to lock it. When installing a new extension, the directory returned by new ExtensionStorage(extensionName).getExtensionDir() does not exist yet, which will cause lock() to throw an ENOENT error and fail the installation.
To resolve this, we should create and lock a stable placeholder file inside the locksDir (which is guaranteed to exist since we call mkdir on it) instead of locking the non-existent extension directory.
for (const extensionName of uniqueNames) {
const lockTarget = path.join(locksDir, extensionName);
await fs.promises.writeFile(lockTarget, '', { flag: 'a' });
releases.push(
await lock(lockTarget, {
realpath: false,
// Keep lock directories outside the extensions directory because the
// loader treats every entry there as a potential extension.
lockfilePath: path.join(locksDir, extensionName + '.lock'),
}),
);
}1 participant
Summary
Prevent two Gemini CLI processes from installing or updating the same extension at the same time.
Without coordination, both processes can pass the destination check and then interleave file copies and metadata writes. This change uses the existing
proper-lockfiledependency to give one process exclusive access to each affected extension name.Details
~/.gemini/extension-locksso extension discovery never treats them as installed extensions.Related Issues
Fixes #29036
How to Validate
npm test -w @google/gemini-cli -- src/config/extension-manager.test.ts src/config/extensions/extensionUpdates.test.ts src/config/extensions/update.test.ts src/commands/extensions/update.test.ts npm run typecheck -w @google/gemini-cli npx eslint packages/cli/src/config/extension-manager.ts packages/cli/src/config/extension-manager.test.ts packages/cli/src/config/extensions/storage.ts packages/cli/src/config/extensions/extensionUpdates.test.ts --max-warnings 0The focused test run passes 46 tests across four files.
Pre-Merge Checklist