fix: Add functions to clean Git lock files locally and remotely by tomchon · Pull Request #152 · taosdata/.github

tomchon · 2026-02-04T10:10:33Z

No description provided.

gemini-code-assist · 2026-02-04T10:10:48Z

Summary of Changes

Hello @tomchon, I'm Gemini Code Assist¹! 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 introduces new functionality to proactively clean up Git lock files, both in local and remote repository environments. By ensuring that any lingering .git/*.lock files are removed before repository operations, the changes aim to prevent potential issues and improve the reliability of the test environment preparation process.

Highlights

Local Git Lock File Cleanup: Implemented a new function clean_git_locks to identify and remove local Git lock files (.git/*.lock) within a specified repository path, enhancing local repository preparation reliability.
Remote Git Lock File Cleanup: Introduced clean_git_locks_remote which leverages the paramiko library to connect to a remote host via SSH and remove .git/*.lock files in remote repositories.
Integration into Repository Preparation: The new cleanup functions are integrated into both local (_prepare_repo) and remote (_prepare_repositories_remote) repository preparation steps, ensuring lock files are removed before further operations.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Changelog

.github/scripts/prepare_test_env.py
- Imported the glob module to facilitate pattern-based file searching.
- Introduced a new function clean_git_locks which identifies and removes .git/*.lock files within a specified local repository path.
- Added a new function clean_git_locks_remote that connects to a remote host via SSH (using paramiko) and executes a command to find and remove .git/*.lock files in a remote repository.
- Modified the _prepare_repo method to call clean_git_locks at the beginning of the repository preparation, ensuring a clean state.
- Modified the _prepare_repositories_remote method to call clean_git_locks_remote for both the TDinternal and community repositories before further remote preparation steps.

Activity

No human activity (comments, reviews, or progress updates) has been recorded on this pull request yet.

Using Gemini Code Assist

The 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 /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.

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.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request introduces new functionality to clean Git lock files both locally and remotely, aiming to improve repository health in automated workflows. However, a high-severity command injection vulnerability was identified in the clean_git_locks_remote function due to unsafe shell command construction, and the use of paramiko.AutoAddPolicy() poses a risk of Man-in-the-Middle attacks. Additionally, there are areas for improvement concerning code structure, logging consistency, and error handling, especially within the remote cleaning function.

.github/scripts/prepare_test_env.py

gemini-code-assist · 2026-02-04T10:12:57Z

.github/scripts/prepare_test_env.py

+                os.remove(lock_file)
+                print(f"Removed lock file: {lock_file}")
+            except Exception as e:
+                print(f"Failed to remove {lock_file}: {e}")


Using print statements is inconsistent with the logging approach (logger.info, logger.error) used elsewhere in this file. Please use logger.error for error messages.

Suggested change

print(f"Failed to remove {lock_file}: {e}")

logger.error(f"Failed to remove {lock_file}: {e}")

.github/scripts/prepare_test_env.py

gemini-code-assist · 2026-02-04T10:12:58Z

.github/scripts/prepare_test_env.py

+            stdin, stdout, stderr = ssh.exec_command(cmd)
+            out = stdout.read().decode()
+            err = stderr.read().decode()
+            print(f"[{host}] Cleaned locks in {git_dir}")


Using print statements is inconsistent with the logging approach. Please use logger.info for informational messages.

Suggested change

print(f"[{host}] Cleaned locks in {git_dir}")

logger.info(f"[{host}] Cleaned locks in {git_dir}")

.github/scripts/prepare_test_env.py

Copilot

Pull request overview

This pull request adds two new functions to clean Git lock files, one for local repositories and another for remote repositories accessed via SSH. The changes aim to prevent Git lock file issues during CI/CD operations by cleaning stale lock files before repository operations.

Changes:

Added clean_git_locks() function to remove local .git/*.lock files
Added clean_git_locks_remote() function to remove remote .git/*.lock files via SSH
Integrated both functions into the repository preparation workflow

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-04T10:14:39Z

.github/scripts/prepare_test_env.py

+        git_dir = os.path.join(repo_path, '.git')
+        if not os.path.exists(git_dir):
+            return
+
+        lock_files = glob.glob(os.path.join(git_dir, '*.lock'))
+        for lock_file in lock_files:
+            try:
+                os.remove(lock_file)


The function accepts repo_path but doesn't validate its type or convert it to a string. Looking at line 232 where this function is called, repo_path is a Path object (as seen from self.wk and self.wkc being Path objects at lines 45-46). However, os.path.join() and os.path.exists() work with both Path objects and strings in Python 3, but glob.glob() expects a string pattern. While this may work due to implicit string conversion, it would be more explicit and safer to convert the Path to a string or use Path methods consistently (e.g., Path(repo_path) / '.git').

Suggested change

git_dir = os.path.join(repo_path, '.git')

if not os.path.exists(git_dir):

return

lock_files = glob.glob(os.path.join(git_dir, '*.lock'))

for lock_file in lock_files:

try:

os.remove(lock_file)

git_dir = Path(repo_path) / ".git"

if not git_dir.exists():

return

lock_files = git_dir.glob("*.lock")

for lock_file in lock_files:

try:

lock_file.unlink()

.github/scripts/prepare_test_env.py

Copilot · 2026-02-04T10:14:40Z

.github/scripts/prepare_test_env.py

+                print(f"Removed lock file: {lock_file}")
+            except Exception as e:
+                print(f"Failed to remove {lock_file}: {e}")


The code uses print() for logging messages, which is inconsistent with the logging convention used throughout the rest of the codebase. Other methods in this class use the logger object (e.g., logger.info(), logger.error()) for logging. This should be updated to use logger.info() for informational messages and logger.error() or logger.warning() for error messages to maintain consistency.

.github/scripts/prepare_test_env.py

Copilot · 2026-02-04T10:14:41Z

.github/scripts/prepare_test_env.py


+
        # 2. Prepare TDinternal
+        self.clean_git_locks_remote(host, username, f"{workdir}/TDinternal")


The function calls self.clean_git_locks_remote(host, username, ...) but doesn't provide the required password or key_filename parameters that are needed for SSH authentication. Looking at the host_config structure used elsewhere in the code (e.g., line 349-352), these parameters should be extracted from host_config if available, or the function should handle authentication properly. Without proper authentication parameters, the SSH connection will fail.

Copilot · 2026-02-04T10:14:41Z

.github/scripts/prepare_test_env.py

-
+
+    def clean_git_locks(repo_path):
+        """清理 Git 锁文件"""


The docstring is written in Chinese ("清理 Git 锁文件"), which is inconsistent with the English docstrings used throughout the rest of the codebase. All other docstrings in this file are in English. For consistency and maintainability, this should be changed to English (e.g., "Clean Git lock files").

…ch in new-framework-test.yml

…ucture

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-04T10:35:15Z

.github/workflows/new-framework-test.yml

        with:
          repository: "taosdata/.github"
-          ref: "main"
+          ref: "fix/clean-git-locks"


The workflow is checking out from a feature branch fix/clean-git-locks instead of the main branch. This appears to be a temporary change for testing purposes and should be reverted to main before merging this PR, unless there's a specific reason to reference this branch permanently. Typically, workflows should reference stable branches like main to avoid dependencies on feature branches that may be deleted after merge.

Suggested change

ref: "fix/clean-git-locks"

ref: "main"

Copilot · 2026-02-04T10:35:16Z

.github/scripts/prepare_test_env.py

        )
-
+
+    def clean_git_locks(self, repo_path):


The function accepts a repo_path parameter but doesn't specify its type. Based on usage in _prepare_repo (line 234), this function receives a Path object. Consider adding type hints to make the expected type explicit, consistent with other methods in the class. For example: def clean_git_locks(self, repo_path: Path):.

Copilot · 2026-02-04T10:35:16Z

.github/scripts/prepare_test_env.py

+        :param key_filename: Path to private key file (optional, for key-based login)
+        """
+        ssh = paramiko.SSHClient()
+        ssh.set_missing_host_key_policy(paramiko.AutoAddPolicy())


The AutoAddPolicy allows any SSH host key without verification, which is a security risk. This could make the connection vulnerable to man-in-the-middle attacks. However, I note that the existing _execute_remote_command method (line 346) also uses AutoAddPolicy, so this is consistent with the current codebase pattern. If this pattern is acceptable for your infrastructure (e.g., trusted internal network), this is fine. Otherwise, consider using a more secure approach like RejectPolicy with properly configured known_hosts.

Suggested change

ssh.set_missing_host_key_policy(paramiko.AutoAddPolicy())

# Load known host keys and reject unknown hosts to avoid MITM vulnerabilities

ssh.load_system_host_keys()

user_known_hosts = os.path.expanduser("~/.ssh/known_hosts")

if os.path.exists(user_known_hosts):

ssh.load_host_keys(user_known_hosts)

ssh.set_missing_host_key_policy(paramiko.RejectPolicy())

fix: Add functions to clean Git lock files locally and remotely

d8be706

Copilot AI review requested due to automatic review settings February 4, 2026 10:10

Copilot started reviewing on behalf of tomchon February 4, 2026 10:10 View session

gemini-code-assist bot reviewed Feb 4, 2026

View reviewed changes

Copilot AI reviewed Feb 4, 2026

View reviewed changes

tomchon added 2 commits February 4, 2026 18:17

fix: Update checkout action reference to use fix/clean-git-locks bran…

42f859c

…ch in new-framework-test.yml

fix: Refactor clean_git_locks methods to improve logging and code str…

5e5d711

…ucture

tomchon requested a review from Copilot February 4, 2026 10:31

Copilot started reviewing on behalf of tomchon February 4, 2026 10:31 View session

Copilot AI reviewed Feb 4, 2026

View reviewed changes

fix: Enhance error handling in clean_git_locks_remote method

0193fb1

	print(f"Failed to remove {lock_file}: {e}")
	logger.error(f"Failed to remove {lock_file}: {e}")

	print(f"[{host}] Cleaned locks in {git_dir}")
	logger.info(f"[{host}] Cleaned locks in {git_dir}")



		# 2. Prepare TDinternal
		self.clean_git_locks_remote(host, username, f"{workdir}/TDinternal")

-        ssh.set_missing_host_key_policy(paramiko.AutoAddPolicy())
+        # Load known host keys and reject unknown hosts to avoid MITM vulnerabilities
+        ssh.load_system_host_keys()
+        user_known_hosts = os.path.expanduser("~/.ssh/known_hosts")
+        if os.path.exists(user_known_hosts):
+            ssh.load_host_keys(user_known_hosts)
+        ssh.set_missing_host_key_policy(paramiko.RejectPolicy())

Conversation

tomchon commented Feb 4, 2026

Uh oh!

gemini-code-assist bot commented Feb 4, 2026

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gemini-code-assist bot Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

gemini-code-assist bot Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants