diff --git a/src/__tests__/piece-selection.test.ts b/src/__tests__/piece-selection.test.ts index a05088a..94ab056 100644 --- a/src/__tests__/piece-selection.test.ts +++ b/src/__tests__/piece-selection.test.ts @@ -33,7 +33,19 @@ vi.mock('../infra/config/index.js', async (importOriginal) => { return actual; }); -const { selectPieceFromEntries, selectPieceFromCategorizedPieces } = await import('../features/pieceSelection/index.js'); +const configMock = vi.hoisted(() => ({ + listPieces: vi.fn(), + listPieceEntries: vi.fn(), + loadAllPiecesWithSources: vi.fn(), + getPieceCategories: vi.fn(), + buildCategorizedPieces: vi.fn(), + getCurrentPiece: vi.fn(), + findPieceCategories: vi.fn(() => []), +})); + +vi.mock('../infra/config/index.js', () => configMock); + +const { selectPieceFromEntries, selectPieceFromCategorizedPieces, selectPiece } = await import('../features/pieceSelection/index.js'); describe('selectPieceFromEntries', () => { beforeEach(() => { @@ -231,3 +243,93 @@ describe('selectPieceFromCategorizedPieces', () => { expect(labels.some((l) => l.includes('Dev'))).toBe(false); }); }); + +describe('selectPiece', () => { + const entries: PieceDirEntry[] = [ + { name: 'custom-flow', path: '/tmp/custom.yaml', source: 'user' }, + { name: 'builtin-flow', path: '/tmp/builtin.yaml', source: 'builtin' }, + ]; + + beforeEach(() => { + selectOptionMock.mockReset(); + bookmarkState.bookmarks = []; + configMock.listPieces.mockReset(); + configMock.listPieceEntries.mockReset(); + configMock.loadAllPiecesWithSources.mockReset(); + configMock.getPieceCategories.mockReset(); + configMock.buildCategorizedPieces.mockReset(); + configMock.getCurrentPiece.mockReset(); + }); + + it('should return default piece when no pieces found and fallbackToDefault is true', async () => { + configMock.getPieceCategories.mockReturnValue(null); + configMock.listPieces.mockReturnValue([]); + configMock.getCurrentPiece.mockReturnValue('default'); + + const result = await selectPiece('/cwd'); + + expect(result).toBe('default'); + }); + + it('should return null when no pieces found and fallbackToDefault is false', async () => { + configMock.getPieceCategories.mockReturnValue(null); + configMock.listPieces.mockReturnValue([]); + configMock.getCurrentPiece.mockReturnValue('default'); + + const result = await selectPiece('/cwd', { fallbackToDefault: false }); + + expect(result).toBeNull(); + }); + + it('should prompt selection even when only one piece exists', async () => { + configMock.getPieceCategories.mockReturnValue(null); + configMock.listPieces.mockReturnValue(['only-piece']); + configMock.listPieceEntries.mockReturnValue([ + { name: 'only-piece', path: '/tmp/only-piece.yaml', source: 'user' }, + ]); + configMock.getCurrentPiece.mockReturnValue('only-piece'); + selectOptionMock.mockResolvedValueOnce('only-piece'); + + const result = await selectPiece('/cwd'); + + expect(result).toBe('only-piece'); + expect(selectOptionMock).toHaveBeenCalled(); + }); + + it('should use category-based selection when category config exists', async () => { + const pieceMap = createPieceMap([{ name: 'my-piece', source: 'user' }]); + const categorized: CategorizedPieces = { + categories: [{ name: 'Dev', pieces: ['my-piece'], children: [] }], + allPieces: pieceMap, + missingPieces: [], + }; + + configMock.getPieceCategories.mockReturnValue({ categories: ['Dev'] }); + configMock.loadAllPiecesWithSources.mockReturnValue(pieceMap); + configMock.buildCategorizedPieces.mockReturnValue(categorized); + configMock.getCurrentPiece.mockReturnValue('my-piece'); + + selectOptionMock.mockResolvedValueOnce('__current__'); + + const result = await selectPiece('/cwd'); + + expect(result).toBe('my-piece'); + expect(configMock.buildCategorizedPieces).toHaveBeenCalled(); + }); + + it('should use directory-based selection when no category config', async () => { + configMock.getPieceCategories.mockReturnValue(null); + configMock.listPieces.mockReturnValue(['piece-a', 'piece-b']); + configMock.listPieceEntries.mockReturnValue(entries); + configMock.getCurrentPiece.mockReturnValue('piece-a'); + + selectOptionMock + .mockResolvedValueOnce('custom') + .mockResolvedValueOnce('custom-flow'); + + const result = await selectPiece('/cwd'); + + expect(result).toBe('custom-flow'); + expect(configMock.listPieceEntries).toHaveBeenCalled(); + }); +}); diff --git a/src/__tests__/selectAndExecute-autoPr.test.ts b/src/__tests__/selectAndExecute-autoPr.test.ts index 2aad356..b483e04 100644 --- a/src/__tests__/selectAndExecute-autoPr.test.ts +++ b/src/__tests__/selectAndExecute-autoPr.test.ts @@ -13,9 +13,6 @@ vi.mock('../infra/config/index.js', () => ({ listPieces: vi.fn(() => ['default']), listPieceEntries: vi.fn(() => []), isPiecePath: vi.fn(() => false), - loadAllPiecesWithSources: vi.fn(() => new Map()), - getPieceCategories: vi.fn(() => null), - buildCategorizedPieces: vi.fn(), loadGlobalConfig: vi.fn(() => ({})), })); @@ -60,29 +57,25 @@ vi.mock('../features/pieceSelection/index.js', () => ({ warnMissingPieces: vi.fn(), selectPieceFromCategorizedPieces: vi.fn(), selectPieceFromEntries: vi.fn(), + selectPiece: vi.fn(), })); import { confirm } from '../shared/prompt/index.js'; import { getCurrentPiece, - loadAllPiecesWithSources, - getPieceCategories, - buildCategorizedPieces, + listPieces, } from '../infra/config/index.js'; import { createSharedClone, autoCommitAndPush, summarizeTaskName } from '../infra/task/index.js'; -import { warnMissingPieces, selectPieceFromCategorizedPieces } from '../features/pieceSelection/index.js'; +import { selectPiece } from '../features/pieceSelection/index.js'; import { selectAndExecuteTask, determinePiece } from '../features/tasks/execute/selectAndExecute.js'; const mockConfirm = vi.mocked(confirm); const mockGetCurrentPiece = vi.mocked(getCurrentPiece); -const mockLoadAllPiecesWithSources = vi.mocked(loadAllPiecesWithSources); -const mockGetPieceCategories = vi.mocked(getPieceCategories); -const mockBuildCategorizedPieces = vi.mocked(buildCategorizedPieces); +const mockListPieces = vi.mocked(listPieces); const mockCreateSharedClone = vi.mocked(createSharedClone); const mockAutoCommitAndPush = vi.mocked(autoCommitAndPush); const mockSummarizeTaskName = vi.mocked(summarizeTaskName); -const mockWarnMissingPieces = vi.mocked(warnMissingPieces); -const mockSelectPieceFromCategorizedPieces = vi.mocked(selectPieceFromCategorizedPieces); +const mockSelectPiece = vi.mocked(selectPiece); beforeEach(() => { vi.clearAllMocks(); @@ -121,44 +114,12 @@ describe('resolveAutoPr default in selectAndExecuteTask', () => { expect(autoPrCall![1]).toBe(true); }); - it('should warn only user-origin missing pieces during interactive selection', async () => { - // Given: category selection is enabled and both builtin/user missing pieces exist - mockGetCurrentPiece.mockReturnValue('default'); - mockLoadAllPiecesWithSources.mockReturnValue(new Map([ - ['default', { - source: 'builtin', - config: { - name: 'default', - movements: [], - initialMovement: 'start', - maxMovements: 1, - }, - }], - ])); - mockGetPieceCategories.mockReturnValue({ - pieceCategories: [], - builtinPieceCategories: [], - userPieceCategories: [], - showOthersCategory: true, - othersCategoryName: 'Others', - }); - mockBuildCategorizedPieces.mockReturnValue({ - categories: [], - allPieces: new Map(), - missingPieces: [ - { categoryPath: ['Quick Start'], pieceName: 'default', source: 'builtin' }, - { categoryPath: ['Custom'], pieceName: 'my-missing', source: 'user' }, - ], - }); - mockSelectPieceFromCategorizedPieces.mockResolvedValue('default'); + it('should call selectPiece when no override is provided', async () => { + mockSelectPiece.mockResolvedValue('selected-piece'); - // When const selected = await determinePiece('/project'); - // Then - expect(selected).toBe('default'); - expect(mockWarnMissingPieces).toHaveBeenCalledWith([ - { categoryPath: ['Custom'], pieceName: 'my-missing', source: 'user' }, - ]); + expect(selected).toBe('selected-piece'); + expect(mockSelectPiece).toHaveBeenCalledWith('/project'); }); }); diff --git a/src/__tests__/switchPiece.test.ts b/src/__tests__/switchPiece.test.ts index 44ab51a..4f1857c 100644 --- a/src/__tests__/switchPiece.test.ts +++ b/src/__tests__/switchPiece.test.ts @@ -5,19 +5,13 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; vi.mock('../infra/config/index.js', () => ({ - listPieceEntries: vi.fn(() => []), - loadAllPiecesWithSources: vi.fn(() => new Map()), - getPieceCategories: vi.fn(() => null), - buildCategorizedPieces: vi.fn(), loadPiece: vi.fn(() => null), getCurrentPiece: vi.fn(() => 'default'), setCurrentPiece: vi.fn(), })); vi.mock('../features/pieceSelection/index.js', () => ({ - warnMissingPieces: vi.fn(), - selectPieceFromCategorizedPieces: vi.fn(), - selectPieceFromEntries: vi.fn(), + selectPiece: vi.fn(), })); vi.mock('../shared/ui/index.js', () => ({ @@ -26,65 +20,41 @@ vi.mock('../shared/ui/index.js', () => ({ error: vi.fn(), })); -import { - loadAllPiecesWithSources, - getPieceCategories, - buildCategorizedPieces, -} from '../infra/config/index.js'; -import { - warnMissingPieces, - selectPieceFromCategorizedPieces, -} from '../features/pieceSelection/index.js'; +import { getCurrentPiece, loadPiece, setCurrentPiece } from '../infra/config/index.js'; +import { selectPiece } from '../features/pieceSelection/index.js'; import { switchPiece } from '../features/config/switchPiece.js'; -const mockLoadAllPiecesWithSources = vi.mocked(loadAllPiecesWithSources); -const mockGetPieceCategories = vi.mocked(getPieceCategories); -const mockBuildCategorizedPieces = vi.mocked(buildCategorizedPieces); -const mockWarnMissingPieces = vi.mocked(warnMissingPieces); -const mockSelectPieceFromCategorizedPieces = vi.mocked(selectPieceFromCategorizedPieces); +const mockGetCurrentPiece = vi.mocked(getCurrentPiece); +const mockLoadPiece = vi.mocked(loadPiece); +const mockSetCurrentPiece = vi.mocked(setCurrentPiece); +const mockSelectPiece = vi.mocked(selectPiece); describe('switchPiece', () => { beforeEach(() => { vi.clearAllMocks(); }); - it('should warn only user-origin missing pieces during interactive switch', async () => { - // Given - mockLoadAllPiecesWithSources.mockReturnValue(new Map([ - ['default', { - source: 'builtin', - config: { - name: 'default', - movements: [], - initialMovement: 'start', - maxMovements: 1, - }, - }], - ])); - mockGetPieceCategories.mockReturnValue({ - pieceCategories: [], - builtinPieceCategories: [], - userPieceCategories: [], - showOthersCategory: true, - othersCategoryName: 'Others', - }); - mockBuildCategorizedPieces.mockReturnValue({ - categories: [], - allPieces: new Map(), - missingPieces: [ - { categoryPath: ['Quick Start'], pieceName: 'default', source: 'builtin' }, - { categoryPath: ['Custom'], pieceName: 'my-missing', source: 'user' }, - ], - }); - mockSelectPieceFromCategorizedPieces.mockResolvedValue(null); + it('should call selectPiece with fallbackToDefault: false', async () => { + mockSelectPiece.mockResolvedValue(null); - // When const switched = await switchPiece('/project'); - // Then expect(switched).toBe(false); - expect(mockWarnMissingPieces).toHaveBeenCalledWith([ - { categoryPath: ['Custom'], pieceName: 'my-missing', source: 'user' }, - ]); + expect(mockSelectPiece).toHaveBeenCalledWith('/project', { fallbackToDefault: false }); + }); + + it('should switch to selected piece', async () => { + mockSelectPiece.mockResolvedValue('new-piece'); + mockLoadPiece.mockReturnValue({ + name: 'new-piece', + movements: [], + initialMovement: 'start', + maxMovements: 1, + }); + + const switched = await switchPiece('/project'); + + expect(switched).toBe(true); + expect(mockSetCurrentPiece).toHaveBeenCalledWith('/project', 'new-piece'); }); }); diff --git a/src/features/config/switchPiece.ts b/src/features/config/switchPiece.ts index 5fab782..d2b26c1 100644 --- a/src/features/config/switchPiece.ts +++ b/src/features/config/switchPiece.ts @@ -3,48 +3,23 @@ */ import { - listPieceEntries, - loadAllPiecesWithSources, - getPieceCategories, - buildCategorizedPieces, loadPiece, getCurrentPiece, setCurrentPiece, } from '../../infra/config/index.js'; import { info, success, error } from '../../shared/ui/index.js'; -import { - warnMissingPieces, - selectPieceFromCategorizedPieces, - selectPieceFromEntries, -} from '../pieceSelection/index.js'; +import { selectPiece } from '../pieceSelection/index.js'; /** * Switch to a different piece * @returns true if switch was successful */ export async function switchPiece(cwd: string, pieceName?: string): Promise { - // No piece specified - show selection prompt if (!pieceName) { const current = getCurrentPiece(cwd); info(`Current piece: ${current}`); - const categoryConfig = getPieceCategories(); - let selected: string | null; - if (categoryConfig) { - const allPieces = loadAllPiecesWithSources(cwd); - if (allPieces.size === 0) { - info('No pieces found.'); - selected = null; - } else { - const categorized = buildCategorizedPieces(allPieces, categoryConfig); - warnMissingPieces(categorized.missingPieces.filter((missing) => missing.source === 'user')); - selected = await selectPieceFromCategorizedPieces(categorized, current); - } - } else { - const entries = listPieceEntries(cwd); - selected = await selectPieceFromEntries(entries, current); - } - + const selected = await selectPiece(cwd, { fallbackToDefault: false }); if (!selected) { info('Cancelled'); return false; diff --git a/src/features/pieceSelection/index.ts b/src/features/pieceSelection/index.ts index 689c096..67cfa98 100644 --- a/src/features/pieceSelection/index.ts +++ b/src/features/pieceSelection/index.ts @@ -12,11 +12,18 @@ import { } from '../../infra/config/global/index.js'; import { findPieceCategories, + listPieces, + listPieceEntries, + loadAllPiecesWithSources, + getPieceCategories, + buildCategorizedPieces, + getCurrentPiece, type PieceDirEntry, type PieceCategoryNode, type CategorizedPieces, type MissingPiece, } from '../../infra/config/index.js'; +import { DEFAULT_PIECE_NAME } from '../../shared/constants.js'; /** Top-level selection item: either a piece or a category containing pieces */ export type PieceSelectionItem = @@ -504,3 +511,44 @@ export async function selectPieceFromEntries( const entriesToUse = customEntries.length > 0 ? customEntries : builtinEntries; return selectPieceFromEntriesWithCategories(entriesToUse, currentPiece); } + +export interface SelectPieceOptions { + fallbackToDefault?: boolean; +} + +export async function selectPiece( + cwd: string, + options?: SelectPieceOptions, +): Promise { + const fallbackToDefault = options?.fallbackToDefault !== false; + const categoryConfig = getPieceCategories(); + const currentPiece = getCurrentPiece(cwd); + + if (categoryConfig) { + const allPieces = loadAllPiecesWithSources(cwd); + if (allPieces.size === 0) { + if (fallbackToDefault) { + info(`No pieces found. Using default: ${DEFAULT_PIECE_NAME}`); + return DEFAULT_PIECE_NAME; + } + info('No pieces found.'); + return null; + } + const categorized = buildCategorizedPieces(allPieces, categoryConfig); + warnMissingPieces(categorized.missingPieces.filter((missing) => missing.source === 'user')); + return selectPieceFromCategorizedPieces(categorized, currentPiece); + } + + const availablePieces = listPieces(cwd); + if (availablePieces.length === 0) { + if (fallbackToDefault) { + info(`No pieces found. Using default: ${DEFAULT_PIECE_NAME}`); + return DEFAULT_PIECE_NAME; + } + info('No pieces found.'); + return null; + } + + const entries = listPieceEntries(cwd); + return selectPieceFromEntries(entries, currentPiece); +} diff --git a/src/features/tasks/execute/selectAndExecute.ts b/src/features/tasks/execute/selectAndExecute.ts index 5816921..3014634 100644 --- a/src/features/tasks/execute/selectAndExecute.ts +++ b/src/features/tasks/execute/selectAndExecute.ts @@ -7,13 +7,8 @@ */ import { - getCurrentPiece, listPieces, - listPieceEntries, isPiecePath, - loadAllPiecesWithSources, - getPieceCategories, - buildCategorizedPieces, loadGlobalConfig, } from '../../../infra/config/index.js'; import { confirm } from '../../../shared/prompt/index.js'; @@ -24,63 +19,12 @@ import { createLogger } from '../../../shared/utils/index.js'; import { createPullRequest, buildPrBody, pushBranch } from '../../../infra/github/index.js'; import { executeTask } from './taskExecution.js'; import type { TaskExecutionOptions, WorktreeConfirmationResult, SelectAndExecuteOptions } from './types.js'; -import { - warnMissingPieces, - selectPieceFromCategorizedPieces, - selectPieceFromEntries, -} from '../../pieceSelection/index.js'; +import { selectPiece } from '../../pieceSelection/index.js'; export type { WorktreeConfirmationResult, SelectAndExecuteOptions }; const log = createLogger('selectAndExecute'); -/** - * Select a piece interactively with directory categories and bookmarks. - */ -async function selectPieceWithDirectoryCategories(cwd: string): Promise { - const availablePieces = listPieces(cwd); - const currentPiece = getCurrentPiece(cwd); - - if (availablePieces.length === 0) { - info(`No pieces found. Using default: ${DEFAULT_PIECE_NAME}`); - return DEFAULT_PIECE_NAME; - } - - if (availablePieces.length === 1 && availablePieces[0]) { - return availablePieces[0]; - } - - const entries = listPieceEntries(cwd); - return selectPieceFromEntries(entries, currentPiece); -} - - -/** - * Select a piece interactively with 2-stage category support. - */ -async function selectPiece(cwd: string): Promise { - const categoryConfig = getPieceCategories(); - if (categoryConfig) { - const current = getCurrentPiece(cwd); - const allPieces = loadAllPiecesWithSources(cwd); - if (allPieces.size === 0) { - info(`No pieces found. Using default: ${DEFAULT_PIECE_NAME}`); - return DEFAULT_PIECE_NAME; - } - const categorized = buildCategorizedPieces(allPieces, categoryConfig); - warnMissingPieces(categorized.missingPieces.filter((missing) => missing.source === 'user')); - return selectPieceFromCategorizedPieces(categorized, current); - } - return selectPieceWithDirectoryCategories(cwd); -} - -/** - * Determine piece to use. - * - * - If override looks like a path (isPiecePath), return it directly (validation is done at load time). - * - If override is a name, validate it exists in available pieces. - * - If no override, prompt user to select interactively. - */ export async function determinePiece(cwd: string, override?: string): Promise { if (override) { if (isPiecePath(override)) { diff --git a/src/features/tasks/list/taskActions.ts b/src/features/tasks/list/taskActions.ts index 2315529..8321c0b 100644 --- a/src/features/tasks/list/taskActions.ts +++ b/src/features/tasks/list/taskActions.ts @@ -24,13 +24,11 @@ import { info, success, error as logError, warn, header, blankLine } from '../.. import { createLogger, getErrorMessage } from '../../../shared/utils/index.js'; import { executeTask } from '../execute/taskExecution.js'; import type { TaskExecutionOptions } from '../execute/types.js'; -import { listPieces, getCurrentPiece } from '../../../infra/config/index.js'; -import { DEFAULT_PIECE_NAME } from '../../../shared/constants.js'; import { encodeWorktreePath } from '../../../infra/config/project/sessionStore.js'; +import { selectPiece } from '../../pieceSelection/index.js'; const log = createLogger('list-tasks'); -/** Actions available for a listed branch */ export type ListAction = 'diff' | 'instruct' | 'try' | 'merge' | 'delete'; /** @@ -254,29 +252,6 @@ export function deleteBranch(projectDir: string, item: BranchListItem): boolean } } -/** - * Get the piece to use for instruction. - */ -async function selectPieceForInstruction(projectDir: string): Promise { - const availablePieces = listPieces(projectDir); - const currentPiece = getCurrentPiece(projectDir); - - if (availablePieces.length === 0) { - return DEFAULT_PIECE_NAME; - } - - if (availablePieces.length === 1 && availablePieces[0]) { - return availablePieces[0]; - } - - const options = availablePieces.map((name) => ({ - label: name === currentPiece ? `${name} (current)` : name, - value: name, - })); - - return await selectOption('Select piece:', options); -} - /** * Get branch context: diff stat and commit log from main branch. */ @@ -343,7 +318,7 @@ export async function instructBranch( return false; } - const selectedPiece = await selectPieceForInstruction(projectDir); + const selectedPiece = await selectPiece(projectDir); if (!selectedPiece) { info('Cancelled'); return false;