From 7aabd0cfaba62dacc45c0b2dd6a38ff0bd175108 Mon Sep 17 00:00:00 2001 From: kewton Date: Tue, 31 Mar 2026 00:32:28 +0900 Subject: [PATCH 1/3] fix: recover inactive schedule cron jobs --- src/lib/schedule-manager.ts | 92 +++++++++++++++++++------ tests/unit/lib/schedule-manager.test.ts | 76 ++++++++++++++++++++ 2 files changed, 146 insertions(+), 22 deletions(-) diff --git a/src/lib/schedule-manager.ts b/src/lib/schedule-manager.ts index e90c1f37..13090ed1 100644 --- a/src/lib/schedule-manager.ts +++ b/src/lib/schedule-manager.ts @@ -80,6 +80,46 @@ interface ManagerState { cmateFileCache: Map; } +function isCronJobActive(cronJob: import('croner').Cron): boolean { + try { + if (typeof cronJob.isStopped === 'function') { + return !cronJob.isStopped(); + } + if (typeof cronJob.isRunning === 'function') { + return cronJob.isRunning(); + } + } catch { + return false; + } + + return true; +} + +function createScheduleState( + worktreeId: string, + scheduleId: string, + entry: ScheduleState['entry'] +): ScheduleState { + const cronJob = new Cron(entry.cronExpression, { + paused: false, + protect: true, + }); + + const state: ScheduleState = { + scheduleId, + worktreeId, + cronJob, + isExecuting: false, + entry, + }; + + cronJob.schedule(() => { + void executeSchedule(state); + }); + + return state; +} + // ============================================================================= // Global State (hot reload persistence) // ============================================================================= @@ -168,15 +208,27 @@ async function syncSchedules(): Promise { continue; } + const worktreeScheduleIds: string[] = []; + let hasInactiveState = false; + for (const [scheduleId, state] of manager.schedules) { + if (state.worktreeId !== worktree.id) continue; + worktreeScheduleIds.push(scheduleId); + if (!isCronJobActive(state.cronJob)) { + hasInactiveState = true; + } + } + // If mtime matches cached value, skip DB operations for this worktree if (cachedMtime !== undefined && cachedMtime === mtime) { - // File unchanged - re-add existing schedule IDs to keep them active - for (const [scheduleId, state] of manager.schedules) { - if (state.worktreeId === worktree.id) { + if (!hasInactiveState) { + // File unchanged - re-add existing schedule IDs to keep them active + for (const scheduleId of worktreeScheduleIds) { activeScheduleIds.add(scheduleId); } + continue; } - continue; + + logger.warn('schedule:inactive-state-detected', { worktreeId: worktree.id }); } // Update mtime cache @@ -219,6 +271,19 @@ async function syncSchedules(): Promise { // Check if this schedule already has a running cron job const existingState = manager.schedules.get(scheduleId); if (existingState) { + if (!isCronJobActive(existingState.cronJob)) { + try { + existingState.cronJob.stop(); + } catch { + // Ignore cleanup errors for inactive cron jobs + } + + const recoveredState = createScheduleState(worktree.id, scheduleId, entry); + manager.schedules.set(scheduleId, recoveredState); + logger.warn('schedule:recreated-inactive', { name: entry.name, cron: entry.cronExpression }); + continue; + } + // Update entry if changed existingState.entry = entry; continue; @@ -226,24 +291,7 @@ async function syncSchedules(): Promise { // Create new cron job try { - const cronJob = new Cron(entry.cronExpression, { - paused: false, - protect: true, // Prevent overlapping - }); - - const state: ScheduleState = { - scheduleId, - worktreeId: worktree.id, - cronJob, - isExecuting: false, - entry, - }; - - // Schedule execution - cronJob.schedule(() => { - void executeSchedule(state); - }); - + const state = createScheduleState(worktree.id, scheduleId, entry); manager.schedules.set(scheduleId, state); logger.info('schedule:created', { name: entry.name, cron: entry.cronExpression }); } catch (cronError) { diff --git a/tests/unit/lib/schedule-manager.test.ts b/tests/unit/lib/schedule-manager.test.ts index 37ddb244..748b32a4 100644 --- a/tests/unit/lib/schedule-manager.test.ts +++ b/tests/unit/lib/schedule-manager.test.ts @@ -15,6 +15,8 @@ import { batchUpsertSchedules, } from '../../../src/lib/schedule-manager'; import { getDbInstance } from '../../../src/lib/db/db-instance'; +import { readCmateFile, parseSchedulesSection } from '../../../src/lib/cmate-parser'; +import { getAllWorktrees, getCmateMtime, batchUpsertSchedules as mockBatchUpsertSchedules } from '../../../src/lib/cron-parser'; // Mock logger module (Issue #480) const { mockLogger } = vi.hoisted(() => { @@ -46,6 +48,17 @@ vi.mock('fs', async (importOriginal) => { }; }); +vi.mock('../../../src/lib/cron-parser', async (importOriginal) => { + const original = await importOriginal(); + return { + ...original, + getAllWorktrees: vi.fn().mockReturnValue([]), + getCmateMtime: vi.fn().mockReturnValue(12345), + batchUpsertSchedules: vi.fn().mockReturnValue([]), + disableStaleSchedules: vi.fn(), + }; +}); + // Mock db-instance to avoid actual DB operations vi.mock('../../../src/lib/db/db-instance', () => { const Database = require('better-sqlite3'); @@ -370,6 +383,69 @@ describe('schedule-manager', () => { await vi.advanceTimersByTimeAsync(0); expect(isScheduleManagerInitialized()).toBe(true); }); + + it('should recreate inactive schedule state even when CMATE.md mtime is unchanged', async () => { + const db = getDbInstance(); + const now = Date.now(); + const scheduleId = 'sched-recover-inactive'; + const worktreeId = 'wt-recover-inactive'; + const worktreePath = '/tmp/recover-inactive'; + const entry = { + name: 'copilot-analysis', + cronExpression: '*/5 * * * *', + message: 'check model', + cliToolId: 'copilot', + enabled: true, + permission: 'yolo', + }; + + vi.mocked(readCmateFile).mockResolvedValue(new Map([['Schedules', [['row']]]])); + vi.mocked(parseSchedulesSection).mockReturnValue([entry]); + vi.mocked(getAllWorktrees).mockReturnValue([{ id: worktreeId, path: worktreePath }]); + vi.mocked(getCmateMtime).mockReturnValue(12345); + vi.mocked(mockBatchUpsertSchedules).mockReturnValue([scheduleId]); + + db.prepare('INSERT OR IGNORE INTO worktrees (id, name, path, updated_at) VALUES (?, ?, ?, ?)').run( + worktreeId, 'recover-wt', worktreePath, now + ); + db.prepare(` + INSERT OR REPLACE INTO scheduled_executions (id, worktree_id, name, message, cron_expression, cli_tool_id, enabled, created_at, updated_at) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?) + `).run( + scheduleId, worktreeId, entry.name, entry.message, entry.cronExpression, entry.cliToolId, 1, now, now + ); + + initScheduleManager(); + await vi.advanceTimersByTimeAsync(0); + + const inactiveCronJob = { + stop: vi.fn(), + schedule: vi.fn(), + isStopped: vi.fn().mockReturnValue(true), + isRunning: vi.fn().mockReturnValue(false), + }; + + globalThis.__scheduleManagerStates!.schedules.set(scheduleId, { + scheduleId, + worktreeId, + cronJob: inactiveCronJob as unknown as import('croner').Cron, + isExecuting: false, + entry, + }); + globalThis.__scheduleManagerStates!.cmateFileCache.set(worktreePath, 12345); + + await vi.advanceTimersByTimeAsync(POLL_INTERVAL_MS); + + const recoveredState = globalThis.__scheduleManagerStates!.schedules.get(scheduleId); + expect(recoveredState).toBeDefined(); + expect(recoveredState!.cronJob).not.toBe(inactiveCronJob); + expect(inactiveCronJob.stop).toHaveBeenCalled(); + expect(mockLogger.warn).toHaveBeenCalledWith('schedule:inactive-state-detected', { worktreeId }); + expect(mockLogger.warn).toHaveBeenCalledWith('schedule:recreated-inactive', { + name: entry.name, + cron: entry.cronExpression, + }); + }); }); describe('restart recovery', () => { From 123089a2b7450d0a366fbd38029d8ea4c2f905dc Mon Sep 17 00:00:00 2001 From: kewton Date: Tue, 31 Mar 2026 00:36:29 +0900 Subject: [PATCH 2/3] feat: expose active schedule state --- locales/en/schedule.json | 10 +++ locales/ja/schedule.json | 10 +++ .../worktrees/[id]/schedules/active/route.ts | 35 +++++++++++ src/components/worktree/ExecutionLogPane.tsx | 63 ++++++++++++++++++- src/lib/schedule-manager.ts | 48 ++++++++++++++ tests/unit/lib/schedule-manager.test.ts | 62 ++++++++++++++++-- 6 files changed, 223 insertions(+), 5 deletions(-) create mode 100644 src/app/api/worktrees/[id]/schedules/active/route.ts diff --git a/locales/en/schedule.json b/locales/en/schedule.json index 25c99f00..e2162ccb 100644 --- a/locales/en/schedule.json +++ b/locales/en/schedule.json @@ -17,6 +17,16 @@ }, "enabled": "Enabled", "disabled": "Disabled", + "activeSchedulesTitle": "Active Schedules", + "activeSchedulesDescription": "In-memory cron state currently registered by the scheduler.", + "noActiveSchedules": "No schedules are currently registered in memory.", + "nextRun": "Next run", + "agentLabel": "Agent", + "activeState": { + "active": "Active", + "inactive": "Inactive", + "executing": "Executing" + }, "lastRun": "Last run", "cron": "Cron", "exitCode": "Exit", diff --git a/locales/ja/schedule.json b/locales/ja/schedule.json index 3a342ccb..fadadb6a 100644 --- a/locales/ja/schedule.json +++ b/locales/ja/schedule.json @@ -17,6 +17,16 @@ }, "enabled": "有効", "disabled": "無効", + "activeSchedulesTitle": "Active Schedules", + "activeSchedulesDescription": "DB設定ではなく、現在メモリ上で登録されている cron state です。", + "noActiveSchedules": "現在メモリ上で登録されている schedule はありません。", + "nextRun": "次回実行", + "agentLabel": "Agent", + "activeState": { + "active": "稼働中", + "inactive": "停止", + "executing": "実行中" + }, "lastRun": "最終実行", "cron": "Cron式", "exitCode": "終了コード", diff --git a/src/app/api/worktrees/[id]/schedules/active/route.ts b/src/app/api/worktrees/[id]/schedules/active/route.ts new file mode 100644 index 00000000..68d6be8f --- /dev/null +++ b/src/app/api/worktrees/[id]/schedules/active/route.ts @@ -0,0 +1,35 @@ +import { NextResponse } from 'next/server'; +import { getDbInstance } from '@/lib/db/db-instance'; +import { getWorktreeById } from '@/lib/db'; +import { isValidWorktreeId } from '@/lib/security/path-validator'; +import { getActiveSchedulesForWorktree } from '@/lib/schedule-manager'; +import { createLogger } from '@/lib/logger'; + +const logger = createLogger('api/schedules/active'); + +export async function GET( + request: Request, + { params }: { params: { id: string } } +) { + void request; + + try { + if (!isValidWorktreeId(params.id)) { + return NextResponse.json({ error: 'Invalid worktree ID format' }, { status: 400 }); + } + + const db = getDbInstance(); + const worktree = getWorktreeById(db, params.id); + if (!worktree) { + return NextResponse.json({ error: `Worktree '${params.id}' not found` }, { status: 404 }); + } + + const schedules = getActiveSchedulesForWorktree(params.id); + return NextResponse.json({ schedules }, { status: 200 }); + } catch (error) { + logger.error('error-fetching-active-schedules:', { + error: error instanceof Error ? error.message : String(error), + }); + return NextResponse.json({ error: 'Failed to fetch active schedules' }, { status: 500 }); + } +} diff --git a/src/components/worktree/ExecutionLogPane.tsx b/src/components/worktree/ExecutionLogPane.tsx index b7596ddd..8eafdbb7 100644 --- a/src/components/worktree/ExecutionLogPane.tsx +++ b/src/components/worktree/ExecutionLogPane.tsx @@ -53,6 +53,18 @@ interface Schedule { updated_at: number; } +interface ActiveSchedule { + scheduleId: string; + worktreeId: string; + name: string; + cronExpression: string; + cliToolId: string; + enabled: boolean; + isExecuting: boolean; + isCronActive: boolean; + nextRunAt: number | null; +} + export interface ExecutionLogPaneProps { worktreeId: string; className?: string; @@ -104,6 +116,7 @@ export const ExecutionLogPane = memo(function ExecutionLogPane({ const t = useTranslations('schedule'); const [logs, setLogs] = useState([]); const [schedules, setSchedules] = useState([]); + const [activeSchedules, setActiveSchedules] = useState([]); const [isLoading, setIsLoading] = useState(true); const [error, setError] = useState(null); const [expandedLogId, setExpandedLogId] = useState(null); @@ -114,9 +127,10 @@ export const ExecutionLogPane = memo(function ExecutionLogPane({ setError(null); try { - const [logsRes, schedulesRes] = await Promise.all([ + const [logsRes, schedulesRes, activeRes] = await Promise.all([ fetch(`/api/worktrees/${worktreeId}/execution-logs`), fetch(`/api/worktrees/${worktreeId}/schedules`), + fetch(`/api/worktrees/${worktreeId}/schedules/active`), ]); if (logsRes.ok) { @@ -128,6 +142,13 @@ export const ExecutionLogPane = memo(function ExecutionLogPane({ const schedulesData = await schedulesRes.json(); setSchedules(schedulesData.schedules || []); } + + if (activeRes.ok) { + const activeData = await activeRes.json(); + setActiveSchedules(activeData.schedules || []); + } else { + setActiveSchedules([]); + } } catch (err) { setError(err instanceof Error ? err.message : 'Failed to fetch data'); } finally { @@ -191,6 +212,46 @@ export const ExecutionLogPane = memo(function ExecutionLogPane({ {/* Schedules Section */}

{t('title')} ({schedules.length})

+
+
+ + {t('activeSchedulesTitle')} ({activeSchedules.length}) + + + {t('activeSchedulesDescription')} + +
+ {activeSchedules.length === 0 ? ( +

{t('noActiveSchedules')}

+ ) : ( +
+ {activeSchedules.map((schedule) => ( +
+
+ {schedule.name} +
+ + {schedule.isCronActive ? t('activeState.active') : t('activeState.inactive')} + + {schedule.isExecuting && ( + + {t('activeState.executing')} + + )} +
+
+
+ {t('cron')}: {schedule.cronExpression || 'N/A'} + {t('agentLabel')}: {schedule.cliToolId} + {schedule.nextRunAt && ( + {t('nextRun')}: {formatTimestamp(schedule.nextRunAt)} + )} +
+
+ ))} +
+ )} +
{schedules.length === 0 ? (

{t('noSchedulesTitle')}

diff --git a/src/lib/schedule-manager.ts b/src/lib/schedule-manager.ts index 13090ed1..ba01e9d9 100644 --- a/src/lib/schedule-manager.ts +++ b/src/lib/schedule-manager.ts @@ -80,6 +80,18 @@ interface ManagerState { cmateFileCache: Map; } +export interface ActiveScheduleInfo { + scheduleId: string; + worktreeId: string; + name: string; + cronExpression: string; + cliToolId: string; + enabled: boolean; + isExecuting: boolean; + isCronActive: boolean; + nextRunAt: number | null; +} + function isCronJobActive(cronJob: import('croner').Cron): boolean { try { if (typeof cronJob.isStopped === 'function') { @@ -473,3 +485,39 @@ export function getScheduleWorktreeIds(): string[] { } return Array.from(worktreeIds); } + +export function getActiveSchedulesForWorktree(worktreeId: string): ActiveScheduleInfo[] { + const manager = getManagerState(); + const schedules: ActiveScheduleInfo[] = []; + + for (const [scheduleId, state] of manager.schedules) { + if (state.worktreeId !== worktreeId) continue; + + let nextRunAt: number | null = null; + try { + const nextRun = typeof state.cronJob.nextRun === 'function' + ? state.cronJob.nextRun() + : null; + if (nextRun instanceof Date) { + nextRunAt = nextRun.getTime(); + } + } catch { + nextRunAt = null; + } + + schedules.push({ + scheduleId, + worktreeId, + name: state.entry.name, + cronExpression: state.entry.cronExpression, + cliToolId: state.entry.cliToolId, + enabled: state.entry.enabled, + isExecuting: state.isExecuting, + isCronActive: isCronJobActive(state.cronJob), + nextRunAt, + }); + } + + schedules.sort((a, b) => a.name.localeCompare(b.name)); + return schedules; +} diff --git a/tests/unit/lib/schedule-manager.test.ts b/tests/unit/lib/schedule-manager.test.ts index 748b32a4..3ee829d3 100644 --- a/tests/unit/lib/schedule-manager.test.ts +++ b/tests/unit/lib/schedule-manager.test.ts @@ -5,6 +5,8 @@ */ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import Database from 'better-sqlite3'; +import { randomUUID } from 'crypto'; import { initScheduleManager, stopAllSchedules, @@ -13,6 +15,7 @@ import { POLL_INTERVAL_MS, MAX_CONCURRENT_SCHEDULES, batchUpsertSchedules, + getActiveSchedulesForWorktree, } from '../../../src/lib/schedule-manager'; import { getDbInstance } from '../../../src/lib/db/db-instance'; import { readCmateFile, parseSchedulesSection } from '../../../src/lib/cmate-parser'; @@ -39,6 +42,11 @@ vi.mock('../../../src/lib/cmate-parser', () => ({ parseSchedulesSection: vi.fn().mockReturnValue([]), })); +vi.mock('../../../src/lib/job-executor', () => ({ + executeSchedule: vi.fn(), + recoverRunningLogs: vi.fn(), +})); + // Mock fs module - only statSync needed for getCmateMtime() (DJ-005) vi.mock('fs', async (importOriginal) => { const original = await importOriginal(); @@ -61,8 +69,7 @@ vi.mock('../../../src/lib/cron-parser', async (importOriginal) => { // Mock db-instance to avoid actual DB operations vi.mock('../../../src/lib/db/db-instance', () => { - const Database = require('better-sqlite3'); - let db: InstanceType | null = null; + let db: Database.Database | null = null; function getTestDb() { if (!db) { @@ -220,7 +227,6 @@ describe('schedule-manager', () => { it('should INSERT new schedules via direct DB test', () => { const db = getDbInstance(); const now = Date.now(); - const { randomUUID } = require('crypto') as typeof import('crypto'); db.prepare('INSERT OR IGNORE INTO worktrees (id, name, path, updated_at) VALUES (?, ?, ?, ?)').run( 'wt-batch-new', 'batch-new-wt', '/tmp/batch-new', now @@ -271,7 +277,6 @@ describe('schedule-manager', () => { it('should UPDATE existing schedules and preserve IDs via direct DB test', () => { const db = getDbInstance(); const now = Date.now(); - const { randomUUID } = require('crypto') as typeof import('crypto'); db.prepare('INSERT OR IGNORE INTO worktrees (id, name, path, updated_at) VALUES (?, ?, ?, ?)').run( 'wt-batch-update', 'batch-update-wt', '/tmp/batch-update', now @@ -492,4 +497,53 @@ describe('schedule-manager', () => { expect(log.status).toBe('failed'); }); }); + + describe('getActiveSchedulesForWorktree', () => { + it('should expose in-memory schedule state for visualization', () => { + const nextRun = new Date('2026-03-31T01:00:00.000Z'); + const cronJob = { + stop: vi.fn(), + schedule: vi.fn(), + isStopped: vi.fn().mockReturnValue(false), + nextRun: vi.fn().mockReturnValue(nextRun), + }; + + globalThis.__scheduleManagerStates = { + timerId: null, + schedules: new Map([ + ['sched-1', { + scheduleId: 'sched-1', + worktreeId: 'wt-1', + cronJob: cronJob as unknown as import('croner').Cron, + isExecuting: true, + entry: { + name: 'copilot-analysis', + message: 'msg', + cronExpression: '*/5 * * * *', + cliToolId: 'copilot', + enabled: true, + permission: 'yolo', + }, + }], + ]), + initialized: true, + isSyncing: false, + cmateFileCache: new Map(), + }; + + expect(getActiveSchedulesForWorktree('wt-1')).toEqual([ + { + scheduleId: 'sched-1', + worktreeId: 'wt-1', + name: 'copilot-analysis', + cronExpression: '*/5 * * * *', + cliToolId: 'copilot', + enabled: true, + isExecuting: true, + isCronActive: true, + nextRunAt: nextRun.getTime(), + }, + ]); + }); + }); }); From 406fc3026fcc87cf3d6b99527bf99d4c8d788c2a Mon Sep 17 00:00:00 2001 From: kewton Date: Tue, 31 Mar 2026 00:59:20 +0900 Subject: [PATCH 3/3] fix(slash-commands): prevent Copilot builtins from overriding Claude standard commands Remove getCopilotBuiltinCommands() from deduplicateByName() in getSlashCommandGroups() to prevent Copilot-only commands (clear, model, compact, etc.) from replacing Claude standard commands via mergeCommandGroups() SF-1 override. Copilot builtins are now injected only in the API route when cliTool === 'copilot'. Fixes #586 Co-Authored-By: Claude Opus 4.6 (1M context) --- .../worktrees/[id]/slash-commands/route.ts | 13 +++- src/lib/slash-commands.ts | 4 +- tests/unit/slash-commands.test.ts | 74 +++++++++++++++++++ 3 files changed, 86 insertions(+), 5 deletions(-) diff --git a/src/app/api/worktrees/[id]/slash-commands/route.ts b/src/app/api/worktrees/[id]/slash-commands/route.ts index 3107c440..100b32a7 100644 --- a/src/app/api/worktrees/[id]/slash-commands/route.ts +++ b/src/app/api/worktrees/[id]/slash-commands/route.ts @@ -16,9 +16,9 @@ import { NextResponse } from 'next/server'; import type { NextRequest } from 'next/server'; import { getDbInstance } from '@/lib/db/db-instance'; import { getWorktreeById } from '@/lib/db'; -import { getSlashCommandGroups, loadCodexSkills, loadCodexPrompts } from '@/lib/slash-commands'; +import { getSlashCommandGroups, loadCodexSkills, loadCodexPrompts, getCopilotBuiltinCommands } from '@/lib/slash-commands'; import { getStandardCommandGroups } from '@/lib/standard-commands'; -import { mergeCommandGroups, filterCommandsByCliTool } from '@/lib/command-merger'; +import { mergeCommandGroups, filterCommandsByCliTool, groupByCategory } from '@/lib/command-merger'; import { isValidWorktreePath } from '@/lib/security/worktree-path-validator'; import { CLI_TOOL_IDS, type CLIToolType } from '@/lib/cli-tools/types'; import type { SlashCommandGroup } from '@/types/slash-commands'; @@ -111,7 +111,14 @@ export async function GET( const globalCodexGroups: SlashCommandGroup[] = allGlobalCodex.length > 0 ? [{ category: 'skill' as const, label: 'Skills', commands: allGlobalCodex }] : []; - const mergedGroups = mergeCommandGroups(standardGroups, [...worktreeGroups, ...globalCodexGroups]); + + // Issue #586: Copilot builtins are only injected when cliTool is 'copilot' + // to prevent overriding Claude standard commands with same names (clear, model, etc.) + const copilotBuiltinGroups: SlashCommandGroup[] = cliTool === 'copilot' + ? groupByCategory(getCopilotBuiltinCommands()) + : []; + + const mergedGroups = mergeCommandGroups(standardGroups, [...worktreeGroups, ...globalCodexGroups, ...copilotBuiltinGroups]); // Issue #4: Filter by CLI tool const filteredGroups = filterCommandsByCliTool(mergedGroups, cliTool); diff --git a/src/lib/slash-commands.ts b/src/lib/slash-commands.ts index e13b628d..5df1e910 100644 --- a/src/lib/slash-commands.ts +++ b/src/lib/slash-commands.ts @@ -525,7 +525,7 @@ export async function getSlashCommandGroups(basePath?: string): Promise []); } - const deduplicated = deduplicateByName([...skillsCache, ...getCopilotBuiltinCommands()], commandsCache); + const deduplicated = deduplicateByName([...skillsCache], commandsCache); return groupByCategory(deduplicated); } diff --git a/tests/unit/slash-commands.test.ts b/tests/unit/slash-commands.test.ts index 116bcd0a..9358bb0b 100644 --- a/tests/unit/slash-commands.test.ts +++ b/tests/unit/slash-commands.test.ts @@ -1216,3 +1216,77 @@ describe('getCopilotBuiltinCommands', () => { expect(commands.length).toBeGreaterThanOrEqual(40); }); }); + +describe('Issue #586: Copilot builtins must not override Claude standard commands', () => { + beforeEach(() => { + vi.resetModules(); + }); + + afterEach(() => { + vi.clearAllMocks(); + }); + + it('getSlashCommandGroups should NOT include Copilot builtin commands in output', async () => { + // When getSlashCommandGroups is called (with or without basePath), + // the returned groups should NOT contain any commands with cliTools: ['copilot'] + // and source: 'builtin'. Copilot builtins should be injected only in the API route. + const testDir = path.resolve(__dirname, '../fixtures/test-586-no-copilot'); + const commandsDir = path.join(testDir, '.claude', 'commands'); + try { + fs.mkdirSync(commandsDir, { recursive: true }); + fs.writeFileSync( + path.join(commandsDir, 'test-cmd.md'), + '---\ndescription: Test command\n---\nContent' + ); + + const { getSlashCommandGroups } = await import('@/lib/slash-commands'); + const groups = await getSlashCommandGroups(testDir); + + const allCommands = groups.flatMap((g) => g.commands); + const copilotBuiltins = allCommands.filter( + (c) => c.source === 'builtin' && c.cliTools?.includes('copilot') + ); + expect(copilotBuiltins).toHaveLength(0); + } finally { + fs.rmSync(testDir, { recursive: true, force: true }); + } + }); + + it('getSlashCommandGroups without basePath should NOT include Copilot builtins', async () => { + const { getSlashCommandGroups, clearCache } = await import('@/lib/slash-commands'); + clearCache(); + const groups = await getSlashCommandGroups(); + + const allCommands = groups.flatMap((g) => g.commands); + const copilotBuiltins = allCommands.filter( + (c) => c.source === 'builtin' && c.cliTools?.includes('copilot') + ); + expect(copilotBuiltins).toHaveLength(0); + }); + + it('deduplicateByName should not be called with Copilot builtins - commands with same names should keep original cliTools', async () => { + // Verify that Claude standard command names (clear, model, compact, etc.) + // are not overwritten by Copilot builtins in deduplicateByName + const { deduplicateByName } = await import('@/lib/slash-commands'); + + const claudeCommands: SlashCommand[] = [ + { name: 'clear', description: 'Clear conversation', category: 'standard-session', filePath: '' }, + { name: 'model', description: 'Switch model', category: 'standard-config', filePath: '' }, + ]; + + const copilotBuiltins: SlashCommand[] = [ + { name: 'clear', description: 'Copilot clear', category: 'standard-session', cliTools: ['copilot'], filePath: '', source: 'builtin' }, + { name: 'model', description: 'Copilot model', category: 'standard-config', cliTools: ['copilot'], filePath: '', source: 'builtin' }, + ]; + + // If Copilot builtins are passed as skills (first arg), they get overridden by commands (second arg) + // But the bug was that they were in skills, and then mergeCommandGroups SF-1 override caused issues. + // The fix ensures Copilot builtins are never in deduplicateByName at all. + const result = deduplicateByName(copilotBuiltins, claudeCommands); + + // Claude commands should take priority + const clearCmd = result.find((c) => c.name === 'clear'); + expect(clearCmd?.cliTools).toBeUndefined(); // Claude's version has no cliTools + expect(clearCmd?.description).toBe('Clear conversation'); + }); +});