-
Notifications
You must be signed in to change notification settings - Fork 2
feat: add cancel command to CLI #44
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
kangju2000
commented
Aug 2, 2025
- Add cancel command implementation to mark active intents as cancelled
- Add cancel command to CLI interface
- Follow same pattern as existing finish
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary of Changes
Hello @kangju2000, 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 introduces a new cancel command to the CLI, enabling users to manage their active intents by explicitly marking them as cancelled. This enhancement provides a crucial capability for controlling intent lifecycles, following the established pattern of existing commands like finish.
Highlights
- New CLI Command: I've added a new
cancelcommand to the command-line interface. This command allows users to explicitly mark their active intents as cancelled, providing a direct way to manage ongoing processes. - Core Logic for Intent Cancellation: I've implemented the core logic for intent cancellation in a new
cancelfunction. This function identifies the active intent associated with a given branch and updates its status to 'cancelled' within the database.
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 or fill out our survey to provide feedback.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a cancel command to the CLI, allowing users to mark the currently active intent as 'cancelled'. The implementation adds a new command to the commander interface and a corresponding core logic function to update the database.
The changes are well-structured and follow the existing patterns in the codebase. I've identified a couple of areas for improvement in the new cancel.ts file:
- The error handling in the
cancelcommand is redundant and can lead to duplicated error messages. I've suggested simplifying it to let the CLI layer handle error presentation. - The use of a hardcoded string for the 'cancelled' status could be improved by using a shared constant to enhance maintainability.
Overall, this is a solid addition. Addressing these points will make the code more robust and maintainable.
| export async function cancel({ | ||
| branchId, | ||
| }: { | ||
| branchId: string; | ||
| }): Promise<Intent> { | ||
| try { | ||
| const activeIntent = await findActiveIntent({ branchId }); | ||
|
|
||
| if (!activeIntent) { | ||
| throw new Error("No active intent found for this branch"); | ||
| } | ||
|
|
||
| return db | ||
| .update(intents) | ||
| .set({ | ||
| status: "cancelled", | ||
| }) | ||
| .where(eq(intents.id, activeIntent.id)) | ||
| .returning() | ||
| .get(); | ||
| } catch (error) { | ||
| console.error("Failed to cancel intent: ", error); | ||
| throw new Error( | ||
| `Failed to cancel intent: ${error instanceof Error ? error.message : "Unknown error"}`, | ||
| ); | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The try...catch block in this function leads to redundant error messages and includes side effects (logging) that are better handled by the caller.
- Redundant Error Messages: The
catchblock throws a new error with the messageFailed to cancel intent: .... The caller insrc/cli.tsalso catches this error and prepends its ownFailed to cancel intent:message, resulting in a duplicated and confusing output for the user (e.g.,Failed to cancel intent: Failed to cancel intent: No active intent found...). - Side Effects: The
console.errorcall within this core function is a side effect that makes it less reusable and testable. It's better practice for library-like functions to be pure and not interact with the console. The CLI layer is the appropriate place to handle user-facing output.
By removing the try...catch block, you simplify the code, avoid the duplicated error message, and improve the separation of concerns. Errors from findActiveIntent or db.update will naturally propagate to the CLI handler, which is already set up to manage them correctly.
export async function cancel({
branchId,
}: {
branchId: string;
}): Promise<Intent> {
const activeIntent = await findActiveIntent({ branchId });
if (!activeIntent) {
throw new Error("No active intent found for this branch");
}
return db
.update(intents)
.set({
status: "cancelled",
})
.where(eq(intents.id, activeIntent.id))
.returning()
.get();
}| return db | ||
| .update(intents) | ||
| .set({ | ||
| status: "cancelled", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The status "cancelled" is hardcoded here. The database schema (src/core/db/schema.ts) refers to an INTENT_STATUS constant for the status column, which suggests a centralized place for these values. Using a hardcoded string here can lead to inconsistencies and maintenance issues if the status values ever change.
To improve maintainability and type safety, it would be better to use a shared constant or enum for intent statuses throughout the application, instead of raw strings. This would apply here and in other places like findActiveIntent.ts.
jcha0713
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one looks good as well. Let's get it merged.