Skip to content

Commit 740cb6c

Browse files
author
Georg Graf
committed
fix(bot): address all 7 Copilot review suggestions for /session_delete (#81)
- Add busy-guard to callback handler - Full session cleanup when deleting current session (detach, stop events, clear pinned) - Import from session/manager for consistency - Handle session.get error separately from not-found - Truncate session title for callback toast (200-char limit) - Answer callback query before deleting message in error paths
1 parent 1481c66 commit 740cb6c

1 file changed

Lines changed: 33 additions & 8 deletions

File tree

src/bot/commands/session-delete.ts

Lines changed: 33 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,16 @@
11
import { CommandContext, Context, InlineKeyboard } from "grammy";
22
import { getDateLocale, t } from "../../i18n/index.js";
33
import { opencodeClient } from "../../opencode/client.js";
4-
import {
5-
clearSession,
6-
getCurrentProject,
7-
getCurrentSession,
8-
} from "../../settings/manager.js";
4+
import { getCurrentProject } from "../../settings/manager.js";
5+
import { clearSession, getCurrentSession } from "../../session/manager.js";
96
import { interactionManager } from "../../interaction/manager.js";
107
import type { InteractionState } from "../../interaction/types.js";
118
import { isForegroundBusy, replyBusyBlocked } from "../utils/busy-guard.js";
129
import { logger } from "../../utils/logger.js";
1310
import { config } from "../../config.js";
11+
import { detachAttachedSession } from "../../attach/service.js";
12+
import { pinnedMessageManager } from "../../pinned/manager.js";
13+
import { summaryAggregator } from "../../summary/aggregator.js";
1414

1515
const CALLBACK_PREFIX = "sesdel:";
1616
const OPEN_PREFIX = `${CALLBACK_PREFIX}open:`;
@@ -115,6 +115,10 @@ function parseSessionPageCallback(data: string): number | null {
115115
return page;
116116
}
117117

118+
function truncateTitle(title: string, maxLength: number = 120): string {
119+
return title.length <= maxLength ? title : `${title.slice(0, maxLength - 3)}...`;
120+
}
121+
118122
async function loadSessionPage(
119123
directory: string,
120124
page: number,
@@ -233,6 +237,11 @@ export async function sessionDeleteCommand(ctx: CommandContext<Context>): Promis
233237
}
234238

235239
export async function handleSessionDeleteCallback(ctx: Context): Promise<boolean> {
240+
if (isForegroundBusy()) {
241+
await replyBusyBlocked(ctx);
242+
return true;
243+
}
244+
236245
const data = ctx.callbackQuery?.data;
237246
if (!data || !data.startsWith(CALLBACK_PREFIX)) {
238247
return false;
@@ -278,11 +287,17 @@ export async function handleSessionDeleteCallback(ctx: Context): Promise<boolean
278287
return true;
279288
}
280289

281-
const { data: session } = await opencodeClient.session.get({
290+
const { data: session, error } = await opencodeClient.session.get({
282291
sessionID: sessionId,
283292
directory: currentProject.worktree,
284293
});
285294

295+
if (error) {
296+
logger.error("[SessionDelete] Failed to fetch session details:", error);
297+
await ctx.answerCallbackQuery({ text: t("session_delete.load_error") });
298+
return true;
299+
}
300+
286301
if (!session) {
287302
clearSessionDeleteInteraction("session_delete_not_found");
288303
await ctx.answerCallbackQuery({ text: t("session_delete.inactive_callback"), show_alert: true }).catch(() => {});
@@ -330,6 +345,7 @@ export async function handleSessionDeleteCallback(ctx: Context): Promise<boolean
330345
const currentProject = getCurrentProject();
331346
if (!currentProject) {
332347
clearSessionDeleteInteraction("session_delete_page_no_project");
348+
await ctx.answerCallbackQuery({ text: t("session_delete.cancelled_callback") }).catch(() => {});
333349
await ctx.deleteMessage().catch(() => {});
334350
return true;
335351
}
@@ -380,6 +396,7 @@ export async function handleSessionDeleteCallback(ctx: Context): Promise<boolean
380396
const currentProject = getCurrentProject();
381397
if (!currentProject) {
382398
clearSessionDeleteInteraction("session_delete_confirm_no_project");
399+
await ctx.answerCallbackQuery({ text: t("session_delete.cancelled_callback") }).catch(() => {});
383400
await ctx.deleteMessage().catch(() => {});
384401
return true;
385402
}
@@ -397,16 +414,24 @@ export async function handleSessionDeleteCallback(ctx: Context): Promise<boolean
397414

398415
const currentSession = getCurrentSession();
399416
const isCurrent = currentSession?.id === sessionId;
417+
const shortTitle = truncateTitle(metadata.title);
400418

401419
logger.info(`[SessionDelete] Session deleted: id=${sessionId}, title="${metadata.title}", wasCurrent=${isCurrent}`);
402420

403421
if (isCurrent) {
422+
detachAttachedSession("session_delete_deleted_current");
404423
clearSession();
424+
summaryAggregator.clear();
405425
clearSessionDeleteInteraction("session_delete_deleted_current");
406-
await ctx.answerCallbackQuery({ text: t("session_delete.deleted_current", { title: metadata.title }) });
426+
try {
427+
await pinnedMessageManager.clear();
428+
} catch (err) {
429+
logger.error("[SessionDelete] Failed to clear pinned message:", err);
430+
}
431+
await ctx.answerCallbackQuery({ text: t("session_delete.deleted_current", { title: shortTitle }) });
407432
} else {
408433
clearSessionDeleteInteraction("session_delete_deleted");
409-
await ctx.answerCallbackQuery({ text: t("session_delete.deleted", { title: metadata.title }) });
434+
await ctx.answerCallbackQuery({ text: t("session_delete.deleted", { title: shortTitle }) });
410435
}
411436

412437
await ctx.deleteMessage().catch(() => {});

0 commit comments

Comments
 (0)