Feat: tag & upgrade & version & fastcommit #6
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @kooksee, 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 significantly enhances the fastcommit tool by introducing robust self-upgrade functionality, expanding its command set with configuration and history management, and refining its core versioning and commit generation processes. The changes aim to provide a more feature-rich, user-friendly, and maintainable application through extensive refactoring, dependency updates, and improved architectural support.
Highlights
- Self-Upgrade Capability: Introduced a new
upgradecommand that allows the tool to self-update by listing and downloading releases from GitHub, complete with a progress bar. - Enhanced Version Management: Refactored the tag generation and push logic, including better handling for
alpha,beta, andreleaseversions, and added checks for remote tag existence. - New Commands: Added
configcommand for managing configuration files and ahistorycommand for interactive shell history management using abubbleteaUI. - Fast Commit Mode: Implemented a
--fastflag for thefastcommitcommand, enabling quick updates and pushes without interactive prompts. - Build System Improvements: Updated
.goreleaser.yamlto supportdarwin_arm64andlinux_arm64architectures, and refined archive naming conventions. - Dependency Updates & Go Version: Upgraded the Go version to
1.24and updated numerous dependencies, including UI libraries (charmbracelet/bubbletea,yarlson/tap), GitHub API client, and logging frameworks. - Refactored Core Utilities: Centralized configuration initialization, improved
gitutility functions withcontext.Contextsupport, and introducedSpinutility for better user feedback during long-running operations.
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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| 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 issue 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.
Footnotes
-
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. ↩
There was a problem hiding this comment.
Code Review
This pull request introduces a significant number of features and refactorings, including new commands for upgrade, tag, config, and history, as well as a fastcommit option. The changes bring substantial improvements, such as better user experience with spinners, enhanced error handling using a result type, proper context propagation, and a more robust command-line structure. However, I've identified a few critical and high-severity issues that should be addressed. These include a hardcoded file path that breaks a feature for other users, a command injection vulnerability, and a potentially destructive configuration update mechanism that could lead to data loss for users. My detailed comments and suggestions for fixes are provided below.
| Action: func(ctx context.Context, command *cli.Command) error { | ||
| defer recovery.Exit() | ||
|
|
||
| var path = "/Users/barry/Documents/git/zshrc.history" |
There was a problem hiding this comment.
The file path for the shell history is hardcoded to a specific user's directory (/Users/barry/...). This will cause the history command to fail for all other users. This path should be made configurable or determined dynamically based on the user's environment (e.g., by reading the $HISTFILE environment variable or checking standard locations for shell history files).
| if cfg.Version == nil || cfg.Version.Name == "" || defaultCfg.Version.Name != cfg.Version.Name { | ||
| assert.Must(os.WriteFile(configPath, defaultConfigData, 0644)) | ||
| } |
There was a problem hiding this comment.
This logic overwrites the entire user configuration file with the default one if the version name in the config differs from the default. This is a destructive action that can lead to the loss of user-specific settings, such as their OpenAI API key or model preferences. A better approach would be to merge the configurations while preserving user settings, or at least to warn the user and ask for confirmation before overwriting the file. I suggest replacing this with a non-destructive warning.
| if cfg.Version == nil || cfg.Version.Name == "" || defaultCfg.Version.Name != cfg.Version.Name { | |
| assert.Must(os.WriteFile(configPath, defaultConfigData, 0644)) | |
| } | |
| if cfg.Version == nil || cfg.Version.Name == "" || defaultCfg.Version.Name != cfg.Version.Name { | |
| log.Warn().Msg("Your configuration file version is outdated. Please consider backing it up and regenerating it to get the latest options.") | |
| } |
| shell := fmt.Sprintf(`%s "%s"`, cmd, path) | ||
| log.Info().Msgf("edit config: %s", shell) | ||
| _, err := script.Exec(shell).Stdout() | ||
| return err |
There was a problem hiding this comment.
The way the shell command is constructed and executed is vulnerable to command injection. Since cmd is taken directly from user arguments and script.Exec passes the string to a shell, a malicious argument like vim; rm -rf / could be executed. You should use os/exec to run the editor, passing the command and the file path as separate arguments to avoid shell interpretation. This will also fix the issue with interactive editors, as .Stdout() prevents interactivity. I see you've already refactored utils.RunOutput to use os/exec; a similar approach should be applied here. You will need to import the os/exec package.
shell := fmt.Sprintf(`%s "%s"`, cmd, path)
log.Info().Msgf("edit config: %s", shell)
editorCmd := exec.CommandContext(ctx, cmd, path)
editorCmd.Stdin = os.Stdin
editorCmd.Stdout = os.Stdout
editorCmd.Stderr = os.Stderr
return editorCmd.Run()| genFile.Const().Id("CommitID").Op("=").Lit("123") | ||
| genFile.Const().Id("BuildTime").Op("=").Lit(time.Now().UTC().Format(time.RFC3339)) | ||
| genFile.Const().Id("Version").Op("=").Lit(strings.TrimSpace(version)) | ||
| genFile.Const().Id("Branch").Op("=").Lit(strings.TrimSpace(utils.GetCurrentBranch().Must())) | ||
| genFile.Const().Id("Project").Op("=").Lit("ffff") |
There was a problem hiding this comment.
This code generator contains hardcoded values for CommitID and Project. The commented-out code suggests it's intended to get these values dynamically (e.g., from git). Additionally, this generator doesn't seem to be used in the Makefile or any build process, making it dead code. If this is intended for future use, it should be completed to fetch dynamic values. If not, it should be removed to avoid confusion.
No description provided.