feat: add cherry-pick workflow to finish command#5
Conversation
fammasmaz-agent
bot
commented
Jan 10, 2026
Adds --cherry-pick (-cp) flag to 'cry finish' as an alternative to the PR workflow. When used, commits are cherry-picked directly to the base branch in the main worktree instead of creating a pull request. Changes: - Added getCommitsAhead(), cherryPick(), cherryPickAbort() functions to src/lib/git.ts - Added performCherryPickToBase() function to src/commands/finish.ts - Added --cherry-pick/-cp CLI flag to the finish command - Integrated cherry-pick workflow with dry-run support Closes #4
|
Starting my review of the cherry-pick workflow addition. This looks like a substantial feature with changes across the CLI, finish command, and git utilities. Diving in now. |
There was a problem hiding this comment.
Overall Assessment
This PR adds a well-structured cherry-pick workflow to the finish command, providing an alternative to the PR-based workflow. The implementation correctly handles conflict detection, cleanup on failure, and integrates smoothly with the existing session management system.
The core logic is sound—commits are retrieved in the correct order for cherry-picking (oldest to newest via --reverse), dirty worktree checks are in place, and the original branch is restored on failure. The dry-run mode is also properly implemented.
Architectural Feedback
The approach of adding cherry-pick as an alternative path within finish() rather than a separate command makes sense, as it shares the same session context and cleanup flow. The new git utility functions (getCommitsAhead, cherryPick, cherryPickAbort) are clean and follow the existing patterns in git.ts.
One consideration: the relationship between --cherry-pick, --pr, and --merge flags could benefit from clearer documentation about which combinations are valid and what takes precedence.
Key Suggestions
- Unused parameter:
sessionBranchinperformCherryPickToBase()is never used—consider removing or marking with underscore - Flag precedence: Document or validate mutual exclusivity between
--cherry-pickand--pr - CLI convention: The
-cpshort flag is unconventional (two characters)—consider alternatives
Nitpicks and Minor Points
- The version bump from 1.0.3 to 2.0.0 is appropriate for this feature addition
- The quote style changes (single → double) add noise to the diff but are a valid style choice
- Consider adding the new
--cherry-pickoption to the README or help documentation
Questions for the Author
- Is there a specific reason
sessionBranchis passed toperformCherryPickToBaseeven though it's unused? Planning for future functionality? - Should there be validation preventing
--cherry-pickand--mergefrom being used together, or is that combination intentionally supported?
This review was generated by an AI assistant.
| function performCherryPickToBase( | ||
| mainRepoRoot: string, | ||
| sessionBranch: string, | ||
| baseBranch: string, | ||
| commits: Array<{ sha: string; message: string }>, | ||
| dryRun: boolean, | ||
| ): { success: boolean; error?: string } { |
There was a problem hiding this comment.
The sessionBranch parameter is declared but never used within this function. Consider either removing it or prefixing with underscore to indicate intentional non-use.
| function performCherryPickToBase( | |
| mainRepoRoot: string, | |
| sessionBranch: string, | |
| baseBranch: string, | |
| commits: Array<{ sha: string; message: string }>, | |
| dryRun: boolean, | |
| ): { success: boolean; error?: string } { | |
| function performCherryPickToBase( | |
| mainRepoRoot: string, | |
| _sessionBranch: string, | |
| baseBranch: string, | |
| commits: Array<{ sha: string; message: string }>, | |
| dryRun: boolean, | |
| ): { success: boolean; error?: string } { |
|
|
||
| if (updatedHasCommits || options.pr) { | ||
| // Cherry-pick workflow (alternative to PR) | ||
| if (options.cherryPick) { |
There was a problem hiding this comment.
When both --cherry-pick and --pr flags are provided, --cherry-pick silently takes precedence due to this condition being checked first. Consider either:
- Adding explicit mutual exclusivity validation with a clear error message, or
- Documenting this precedence behavior in the CLI help text
src/index.ts
Outdated
| .option("--dry-run", "Show what would happen without executing") | ||
| .option("--pr", "Force PR creation path") | ||
| .option( | ||
| "--cherry-pick, -cp", |
There was a problem hiding this comment.
Minor: The short flag -cp is a two-character alias, which is unconventional for CLI tools (most use single-character short flags). Consider using just --cherry-pick without a short alias, or a single-character alternative like -C if it's available and makes sense semantically.
|
@Fmazbot-agent There's a lot of reformatting of code here that is not relevant to the need of the pr. Please undo that.
|
|
@Fmazbot-agent There's a lot of reformatting of code here that is not relevant to the need of the pr. Please undo that. |
|
@FammasMaz, understood. I'll analyze the diff to identify the formatting-only changes (like quote style changes) that aren't related to the cherry-pick feature, and revert them while keeping the functional changes intact. Starting the analysis now. |
- Reverted quote style changes (double quotes back to single quotes) - Reverted multi-line formatting changes - Kept functional cherry-pick feature: - Added getCommitsAhead, cherryPick, cherryPickAbort to git.ts - Added performCherryPickToBase function to finish.ts - Added --cherry-pick CLI option to finish command - Added cherry-pick workflow as alternative to PR creation
|
@FammasMaz, I've successfully removed the reformatting changes while preserving the cherry-pick feature. SummaryThe PR originally contained two types of changes:
I've now reverted all the reformatting changes while keeping the functional cherry-pick feature intact. Changes MadeReverted (reformatting):
Preserved (functional cherry-pick feature):
CommitThe changes were committed in: c79e75f Diff StatsThe net reduction in lines is due to reverting the multi-line formatting that was adding extra lines. This update was generated by an AI assistant. |
|
@fmazbot-agent You need to update docs as well accordingly |
|
@FammasMaz, I'm on it! I'll update the documentation to include the new Since this PR is already merged, I'll create a new branch from master and open a separate PR with the documentation updates. Starting now. |
|
@FammasMaz, I've updated the documentation for the cherry-pick workflow. SummaryCreated a new PR with documentation updates for the Changes Made
Pull Request CreatedReady for your review! This update was generated by an AI assistant. |