Skip to content

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.

Sign up for GitHub

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

Merged
kangju2000 merged 3 commits into from
Aug 2, 2025
Merged

feat: add cancel command to CLI #44

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
706d8fa
feat: add cancel command to terminate active intents
kangju2000 Aug 2, 2025
d784a67
feat: add CLI command to cancel active intent
kangju2000 Aug 2, 2025
d6c9dcb
Merge branch 'next' into feat/cancel-action
kangju2000 Aug 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions src/cli.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,22 @@ program
}
});

program
.command("cancel")
.description("Cancel the active intent")
.action(async () => {
try {
const projectId = await ensureProject();
const branchId = await ensureBranch(projectId);

const intent = await commands.cancel({ branchId });
console.log(`Cancelled intent #${intent.id}: ${intent.message}`);
} catch (error) {
console.error("Failed to cancel intent:", getErrorMessage(error));
process.exit(1);
}
});

program
.command("finish")
.description("Finish the current active intent")
Expand Down
32 changes: 32 additions & 0 deletions src/core/commands/cancel.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import { eq } from "drizzle-orm";
import { db } from "../db";
import { type Intent, intents } from "../db/schema";
import { findActiveIntent } from "./findActiveIntent";

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",
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Aug 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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.

})
.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"}`,
);
}
}
Comment on lines +6 to +32
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Aug 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The try...catch block in this function leads to redundant error messages and includes side effects (logging) that are better handled by the caller.

  1. Redundant Error Messages: The catch block throws a new error with the message Failed to cancel intent: .... The caller in src/cli.ts also catches this error and prepends its own Failed 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...).
  2. Side Effects: The console.error call 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();
}

1 change: 1 addition & 0 deletions src/core/commands/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
export * from "./cancel";
export * from "./findActiveIntent";
export * from "./finish";
export * from "./list";
Expand Down