From e23359b1bf22bcca58a06bd46d6af5cbdf1a703d Mon Sep 17 00:00:00 2001 From: nrs <38722970+nrslib@users.noreply.github.com> Date: Sun, 8 Feb 2026 07:17:17 +0900 Subject: [PATCH 1/3] takt: github-issue-130-tasuku-autopr (#140) --- src/__tests__/addTask.test.ts | 17 +++ src/__tests__/selectAndExecute-autoPr.test.ts | 104 ++++++++++++++++++ src/features/tasks/add/index.ts | 2 +- .../tasks/execute/selectAndExecute.ts | 2 +- 4 files changed, 123 insertions(+), 2 deletions(-) create mode 100644 src/__tests__/selectAndExecute-autoPr.test.ts diff --git a/src/__tests__/addTask.test.ts b/src/__tests__/addTask.test.ts index bd05b25..96b1756 100644 --- a/src/__tests__/addTask.test.ts +++ b/src/__tests__/addTask.test.ts @@ -386,6 +386,23 @@ describe('addTask', () => { expect(mockResolveIssueTask).toHaveBeenCalledWith('#99'); }); + it('should call auto-PR confirm with default true', async () => { + // Given: worktree is confirmed so auto-PR prompt is reached + setupFullFlowMocks({ slug: 'auto-pr-default' }); + mockConfirm.mockResolvedValue(true); + mockPromptInput.mockResolvedValue(''); + + // When + await addTask(testDir); + + // Then: second confirm call (Auto-create PR?) has defaultYes=true + const autoPrCall = mockConfirm.mock.calls.find( + (call) => call[0] === 'Auto-create PR?', + ); + expect(autoPrCall).toBeDefined(); + expect(autoPrCall![1]).toBe(true); + }); + describe('create_issue action', () => { it('should call createIssue when create_issue action is selected', async () => { // Given: interactive mode returns create_issue action diff --git a/src/__tests__/selectAndExecute-autoPr.test.ts b/src/__tests__/selectAndExecute-autoPr.test.ts new file mode 100644 index 0000000..2cce3f2 --- /dev/null +++ b/src/__tests__/selectAndExecute-autoPr.test.ts @@ -0,0 +1,104 @@ +/** + * Tests for resolveAutoPr default behavior in selectAndExecuteTask + */ + +import { describe, it, expect, vi, beforeEach } from 'vitest'; + +vi.mock('../shared/prompt/index.js', () => ({ + confirm: vi.fn(), +})); + +vi.mock('../infra/config/index.js', () => ({ + getCurrentPiece: vi.fn(), + 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(() => ({})), +})); + +vi.mock('../infra/task/index.js', () => ({ + createSharedClone: vi.fn(), + autoCommitAndPush: vi.fn(), + summarizeTaskName: vi.fn(), +})); + +vi.mock('../shared/ui/index.js', () => ({ + info: vi.fn(), + error: vi.fn(), + success: vi.fn(), +})); + +vi.mock('../shared/utils/index.js', async (importOriginal) => ({ + ...(await importOriginal>()), + createLogger: () => ({ + info: vi.fn(), + debug: vi.fn(), + error: vi.fn(), + }), +})); + +vi.mock('../infra/github/index.js', () => ({ + createPullRequest: vi.fn(), + buildPrBody: vi.fn(), + pushBranch: vi.fn(), +})); + +vi.mock('../features/tasks/execute/taskExecution.js', () => ({ + executeTask: vi.fn(), +})); + +vi.mock('../features/pieceSelection/index.js', () => ({ + warnMissingPieces: vi.fn(), + selectPieceFromCategorizedPieces: vi.fn(), + selectPieceFromEntries: vi.fn(), +})); + +import { confirm } from '../shared/prompt/index.js'; +import { createSharedClone, autoCommitAndPush, summarizeTaskName } from '../infra/task/index.js'; +import { selectAndExecuteTask } from '../features/tasks/execute/selectAndExecute.js'; + +const mockConfirm = vi.mocked(confirm); +const mockCreateSharedClone = vi.mocked(createSharedClone); +const mockAutoCommitAndPush = vi.mocked(autoCommitAndPush); +const mockSummarizeTaskName = vi.mocked(summarizeTaskName); + +beforeEach(() => { + vi.clearAllMocks(); +}); + +describe('resolveAutoPr default in selectAndExecuteTask', () => { + it('should call auto-PR confirm with default true when no CLI option or config', async () => { + // Given: worktree is enabled via override, no autoPr option, no global config autoPr + mockConfirm.mockResolvedValue(true); + mockSummarizeTaskName.mockResolvedValue('test-task'); + mockCreateSharedClone.mockReturnValue({ + path: '/project/../clone', + branch: 'takt/test-task', + }); + + const { executeTask } = await import( + '../features/tasks/execute/taskExecution.js' + ); + vi.mocked(executeTask).mockResolvedValue(true); + mockAutoCommitAndPush.mockReturnValue({ + success: false, + message: 'no changes', + }); + + // When + await selectAndExecuteTask('/project', 'test task', { + piece: 'default', + createWorktree: true, + }); + + // Then: the 'Create pull request?' confirm is called with default true + const autoPrCall = mockConfirm.mock.calls.find( + (call) => call[0] === 'Create pull request?', + ); + expect(autoPrCall).toBeDefined(); + expect(autoPrCall![1]).toBe(true); + }); +}); diff --git a/src/features/tasks/add/index.ts b/src/features/tasks/add/index.ts index cb0e6a7..3b92c2a 100644 --- a/src/features/tasks/add/index.ts +++ b/src/features/tasks/add/index.ts @@ -186,7 +186,7 @@ export async function addTask(cwd: string, task?: string): Promise { } // PR確認（worktreeが有効な場合のみ） - autoPr = await confirm('Auto-create PR?', false); + autoPr = await confirm('Auto-create PR?', true); } // YAMLファイル作成 diff --git a/src/features/tasks/execute/selectAndExecute.ts b/src/features/tasks/execute/selectAndExecute.ts index 03f97ac..715609b 100644 --- a/src/features/tasks/execute/selectAndExecute.ts +++ b/src/features/tasks/execute/selectAndExecute.ts @@ -141,7 +141,7 @@ async function resolveAutoPr(optionAutoPr: boolean | undefined): Promise Date: Sun, 8 Feb 2026 07:20:36 +0900 Subject: [PATCH 2/3] takt: github-issue-114-debaggu-puron (#139) --- src/__tests__/debug.test.ts | 80 ++++++++ src/__tests__/it-sigint-interrupt.test.ts | 2 + .../pieceExecution-debug-prompts.test.ts | 190 ++++++++++++++++++ src/features/tasks/execute/pieceExecution.ts | 37 +++- src/shared/utils/debug.ts | 43 +++- src/shared/utils/types.ts | 10 + 6 files changed, 356 insertions(+), 6 deletions(-) create mode 100644 src/__tests__/pieceExecution-debug-prompts.test.ts diff --git a/src/__tests__/debug.test.ts b/src/__tests__/debug.test.ts index 81adcd8..05aea16 100644 --- a/src/__tests__/debug.test.ts +++ b/src/__tests__/debug.test.ts @@ -14,11 +14,21 @@ import { debugLog, infoLog, errorLog, + writePromptLog, } from '../shared/utils/index.js'; import { existsSync, readFileSync, mkdirSync, rmSync } from 'node:fs'; import { join } from 'node:path'; import { tmpdir } from 'node:os'; +function resolvePromptsLogFilePath(): string { + const debugLogFile = getDebugLogFile(); + expect(debugLogFile).not.toBeNull(); + if (!debugLogFile!.endsWith('.log')) { + throw new Error(`unexpected debug log file path: ${debugLogFile!}`); + } + return debugLogFile!.replace(/\.log$/, '-prompts.jsonl'); +} + describe('debug logging', () => { beforeEach(() => { resetDebugLogger(); @@ -69,6 +79,21 @@ describe('debug logging', () => { } }); + it('should create prompts log file with -prompts suffix', () => { + const projectDir = join(tmpdir(), 'takt-test-debug-prompts-' + Date.now()); + mkdirSync(projectDir, { recursive: true }); + + try { + initDebugLogger({ enabled: true }, projectDir); + const promptsLogFile = resolvePromptsLogFilePath(); + expect(promptsLogFile).toContain(join(projectDir, '.takt', 'logs')); + expect(promptsLogFile).toMatch(/debug-.*-prompts\.jsonl$/); + expect(existsSync(promptsLogFile)).toBe(true); + } finally { + rmSync(projectDir, { recursive: true, force: true }); + } + }); + it('should not create log file when projectDir is not provided', () => { initDebugLogger({ enabled: true }); expect(isDebugEnabled()).toBe(true); @@ -83,7 +108,9 @@ describe('debug logging', () => { try { initDebugLogger({ enabled: true, logFile }, '/tmp'); expect(getDebugLogFile()).toBe(logFile); + expect(resolvePromptsLogFilePath()).toBe(join(logDir, 'test-prompts.jsonl')); expect(existsSync(logFile)).toBe(true); + expect(existsSync(join(logDir, 'test-prompts.jsonl'))).toBe(true); const content = readFileSync(logFile, 'utf-8'); expect(content).toContain('TAKT Debug Log'); @@ -122,6 +149,59 @@ describe('debug logging', () => { }); }); + describe('writePromptLog', () => { + it('should append prompt log record when debug is enabled', () => { + const projectDir = join(tmpdir(), 'takt-test-debug-write-prompts-' + Date.now()); + mkdirSync(projectDir, { recursive: true }); + + try { + initDebugLogger({ enabled: true }, projectDir); + const promptsLogFile = resolvePromptsLogFilePath(); + + writePromptLog({ + movement: 'plan', + phase: 1, + iteration: 2, + prompt: 'prompt text', + response: 'response text', + timestamp: '2026-02-07T00:00:00.000Z', + }); + + const content = readFileSync(promptsLogFile, 'utf-8').trim(); + expect(content).not.toBe(''); + const parsed = JSON.parse(content) as { + movement: string; + phase: number; + iteration: number; + prompt: string; + response: string; + timestamp: string; + }; + expect(parsed.movement).toBe('plan'); + expect(parsed.phase).toBe(1); + expect(parsed.iteration).toBe(2); + expect(parsed.prompt).toBe('prompt text'); + expect(parsed.response).toBe('response text'); + expect(parsed.timestamp).toBe('2026-02-07T00:00:00.000Z'); + } finally { + rmSync(projectDir, { recursive: true, force: true }); + } + }); + + it('should do nothing when debug is disabled', () => { + writePromptLog({ + movement: 'plan', + phase: 1, + iteration: 1, + prompt: 'ignored prompt', + response: 'ignored response', + timestamp: '2026-02-07T00:00:00.000Z', + }); + + expect(getDebugLogFile()).toBeNull(); + }); + }); + describe('setVerboseConsole / isVerboseConsole', () => { it('should default to false', () => { expect(isVerboseConsole()).toBe(false); diff --git a/src/__tests__/it-sigint-interrupt.test.ts b/src/__tests__/it-sigint-interrupt.test.ts index f9c0df9..7c0944a 100644 --- a/src/__tests__/it-sigint-interrupt.test.ts +++ b/src/__tests__/it-sigint-interrupt.test.ts @@ -128,6 +128,8 @@ vi.mock('../shared/utils/index.js', () => ({ }), notifySuccess: vi.fn(), notifyError: vi.fn(), + isDebugEnabled: vi.fn().mockReturnValue(false), + writePromptLog: vi.fn(), })); vi.mock('../shared/prompt/index.js', () => ({ diff --git a/src/__tests__/pieceExecution-debug-prompts.test.ts b/src/__tests__/pieceExecution-debug-prompts.test.ts new file mode 100644 index 0000000..7d9ca04 --- /dev/null +++ b/src/__tests__/pieceExecution-debug-prompts.test.ts @@ -0,0 +1,190 @@ +/** + * Integration tests: debug prompt log wiring in executePiece(). + */ + +import { describe, it, expect, beforeEach, vi } from 'vitest'; +import type { PieceConfig } from '../core/models/index.js'; + +const { mockIsDebugEnabled, mockWritePromptLog, MockPieceEngine } = vi.hoisted(() => { + // eslint-disable-next-line @typescript-eslint/no-require-imports + const { EventEmitter: EE } = require('node:events') as typeof import('node:events'); + + const mockIsDebugEnabled = vi.fn().mockReturnValue(true); + const mockWritePromptLog = vi.fn(); + + class MockPieceEngine extends EE { + private config: PieceConfig; + + constructor(config: PieceConfig, _cwd: string, _task: string, _options: unknown) { + super(); + this.config = config; + } + + abort(): void {} + + async run(): Promise<{ status: string; iteration: number }> { + const step = this.config.movements[0]!; + const timestamp = new Date('2026-02-07T00:00:00.000Z'); + + this.emit('movement:start', step, 1, 'movement instruction'); + this.emit('phase:start', step, 1, 'execute', 'phase prompt'); + this.emit('phase:complete', step, 1, 'execute', 'phase response', 'done'); + this.emit( + 'movement:complete', + step, + { + persona: step.personaDisplayName, + status: 'done', + content: 'movement response', + timestamp, + }, + 'movement instruction' + ); + this.emit('piece:complete', { status: 'completed', iteration: 1 }); + + return { status: 'completed', iteration: 1 }; + } + } + + return { mockIsDebugEnabled, mockWritePromptLog, MockPieceEngine }; +}); + +vi.mock('../core/piece/index.js', () => ({ + PieceEngine: MockPieceEngine, +})); + +vi.mock('../infra/claude/index.js', () => ({ + callAiJudge: vi.fn(), + detectRuleIndex: vi.fn(), + interruptAllQueries: vi.fn(), +})); + +vi.mock('../infra/config/index.js', () => ({ + loadPersonaSessions: vi.fn().mockReturnValue({}), + updatePersonaSession: vi.fn(), + loadWorktreeSessions: vi.fn().mockReturnValue({}), + updateWorktreeSession: vi.fn(), + loadGlobalConfig: vi.fn().mockReturnValue({ provider: 'claude' }), + saveSessionState: vi.fn(), +})); + +vi.mock('../shared/context.js', () => ({ + isQuietMode: vi.fn().mockReturnValue(true), +})); + +vi.mock('../shared/ui/index.js', () => ({ + header: vi.fn(), + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + success: vi.fn(), + status: vi.fn(), + blankLine: vi.fn(), + StreamDisplay: vi.fn().mockImplementation(() => ({ + createHandler: vi.fn().mockReturnValue(vi.fn()), + flush: vi.fn(), + })), +})); + +vi.mock('../infra/fs/index.js', () => ({ + generateSessionId: vi.fn().mockReturnValue('test-session-id'), + createSessionLog: vi.fn().mockReturnValue({ + startTime: new Date().toISOString(), + iterations: 0, + }), + finalizeSessionLog: vi.fn().mockImplementation((log, status) => ({ + ...log, + status, + endTime: new Date().toISOString(), + })), + updateLatestPointer: vi.fn(), + initNdjsonLog: vi.fn().mockReturnValue('/tmp/test-log.jsonl'), + appendNdjsonLine: vi.fn(), +})); + +vi.mock('../shared/utils/index.js', () => ({ + createLogger: vi.fn().mockReturnValue({ + debug: vi.fn(), + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + }), + notifySuccess: vi.fn(), + notifyError: vi.fn(), + preventSleep: vi.fn(), + isDebugEnabled: mockIsDebugEnabled, + writePromptLog: mockWritePromptLog, +})); + +vi.mock('../shared/prompt/index.js', () => ({ + selectOption: vi.fn(), + promptInput: vi.fn(), +})); + +vi.mock('../shared/i18n/index.js', () => ({ + getLabel: vi.fn().mockImplementation((key: string) => key), +})); + +vi.mock('../shared/exitCodes.js', () => ({ + EXIT_SIGINT: 130, +})); + +import { executePiece } from '../features/tasks/execute/pieceExecution.js'; + +describe('executePiece debug prompts logging', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + function makeConfig(): PieceConfig { + return { + name: 'test-piece', + maxIterations: 5, + initialMovement: 'implement', + movements: [ + { + name: 'implement', + persona: '../agents/coder.md', + personaDisplayName: 'coder', + instructionTemplate: 'Implement task', + passPreviousResponse: true, + rules: [{ condition: 'done', next: 'COMPLETE' }], + }, + ], + }; + } + + it('should write prompt log record when debug is enabled', async () => { + mockIsDebugEnabled.mockReturnValue(true); + + await executePiece(makeConfig(), 'task', '/tmp/project', { + projectCwd: '/tmp/project', + }); + + expect(mockWritePromptLog).toHaveBeenCalledTimes(1); + const record = mockWritePromptLog.mock.calls[0]?.[0] as { + movement: string; + phase: number; + iteration: number; + prompt: string; + response: string; + timestamp: string; + }; + expect(record.movement).toBe('implement'); + expect(record.phase).toBe(1); + expect(record.iteration).toBe(1); + expect(record.prompt).toBe('phase prompt'); + expect(record.response).toBe('phase response'); + expect(record.timestamp).toMatch(/^\d{4}-\d{2}-\d{2}T/); + }); + + it('should not write prompt log record when debug is disabled', async () => { + mockIsDebugEnabled.mockReturnValue(false); + + await executePiece(makeConfig(), 'task', '/tmp/project', { + projectCwd: '/tmp/project', + }); + + expect(mockWritePromptLog).not.toHaveBeenCalled(); + }); +}); diff --git a/src/features/tasks/execute/pieceExecution.ts b/src/features/tasks/execute/pieceExecution.ts index 87c6e21..049ea2b 100644 --- a/src/features/tasks/execute/pieceExecution.ts +++ b/src/features/tasks/execute/pieceExecution.ts @@ -46,7 +46,15 @@ import { type NdjsonInteractiveStart, type NdjsonInteractiveEnd, } from '../../../infra/fs/index.js'; -import { createLogger, notifySuccess, notifyError, preventSleep } from '../../../shared/utils/index.js'; +import { + createLogger, + notifySuccess, + notifyError, + preventSleep, + isDebugEnabled, + writePromptLog, +} from '../../../shared/utils/index.js'; +import type { PromptLogRecord } from '../../../shared/utils/index.js'; import { selectOption, promptInput } from '../../../shared/prompt/index.js'; import { EXIT_SIGINT } from '../../../shared/exitCodes.js'; import { getLabel } from '../../../shared/i18n/index.js'; @@ -241,6 +249,8 @@ export async function executePiece( let abortReason: string | undefined; let lastMovementContent: string | undefined; let lastMovementName: string | undefined; + let currentIteration = 0; + const phasePrompts = new Map(); engine.on('phase:start', (step, phase, phaseName, instruction) => { log.debug('Phase starting', { step: step.name, phase, phaseName }); @@ -253,6 +263,10 @@ export async function executePiece( ...(instruction ? { instruction } : {}), }; appendNdjsonLine(ndjsonLogPath, record); + + if (isDebugEnabled()) { + phasePrompts.set(`${step.name}:${phase}`, instruction); + } }); engine.on('phase:complete', (step, phase, phaseName, content, phaseStatus, phaseError) => { @@ -263,15 +277,34 @@ export async function executePiece( phase, phaseName, status: phaseStatus, - content: content ?? '', + content, timestamp: new Date().toISOString(), ...(phaseError ? { error: phaseError } : {}), }; appendNdjsonLine(ndjsonLogPath, record); + + const promptKey = `${step.name}:${phase}`; + const prompt = phasePrompts.get(promptKey); + phasePrompts.delete(promptKey); + + if (isDebugEnabled()) { + if (prompt) { + const promptRecord: PromptLogRecord = { + movement: step.name, + phase, + iteration: currentIteration, + prompt, + response: content, + timestamp: new Date().toISOString(), + }; + writePromptLog(promptRecord); + } + } }); engine.on('movement:start', (step, iteration, instruction) => { log.debug('Movement starting', { step: step.name, persona: step.personaDisplayName, iteration }); + currentIteration = iteration; info(`[${iteration}/${pieceConfig.maxIterations}] ${step.name} (${step.personaDisplayName})`); // Log prompt content for debugging diff --git a/src/shared/utils/debug.ts b/src/shared/utils/debug.ts index 5716a85..c3f8b7c 100644 --- a/src/shared/utils/debug.ts +++ b/src/shared/utils/debug.ts @@ -6,6 +6,7 @@ import { existsSync, appendFileSync, mkdirSync, writeFileSync } from 'node:fs'; import { dirname, join } from 'node:path'; +import type { PromptLogRecord } from './types.js'; /** Debug configuration (duplicated from core/models to avoid shared → core dependency) */ interface DebugConfig { enabled: boolean; @@ -21,6 +22,7 @@ export class DebugLogger { private debugEnabled = false; private debugLogFile: string | null = null; + private debugPromptsLogFile: string | null = null; private initialized = false; private verboseConsoleEnabled = false; @@ -38,10 +40,10 @@ export class DebugLogger { DebugLogger.instance = null; } - /** Get default debug log file path */ - private static getDefaultLogFile(projectDir: string): string { + /** Get default debug log file prefix */ + private static getDefaultLogPrefix(projectDir: string): string { const timestamp = new Date().toISOString().replace(/[:.]/g, '-').slice(0, 19); - return join(projectDir, '.takt', 'logs', `debug-${timestamp}.log`); + return join(projectDir, '.takt', 'logs', `debug-${timestamp}`); } /** Initialize debug logger from config */ @@ -55,8 +57,15 @@ export class DebugLogger { if (this.debugEnabled) { if (config?.logFile) { this.debugLogFile = config.logFile; + if (config.logFile.endsWith('.log')) { + this.debugPromptsLogFile = config.logFile.slice(0, -4) + '-prompts.jsonl'; + } else { + this.debugPromptsLogFile = `${config.logFile}-prompts.jsonl`; + } } else if (projectDir) { - this.debugLogFile = DebugLogger.getDefaultLogFile(projectDir); + const logPrefix = DebugLogger.getDefaultLogPrefix(projectDir); + this.debugLogFile = `${logPrefix}.log`; + this.debugPromptsLogFile = `${logPrefix}-prompts.jsonl`; } if (this.debugLogFile) { @@ -76,6 +85,14 @@ export class DebugLogger { writeFileSync(this.debugLogFile, header, 'utf-8'); } + + if (this.debugPromptsLogFile) { + const promptsLogDir = dirname(this.debugPromptsLogFile); + if (!existsSync(promptsLogDir)) { + mkdirSync(promptsLogDir, { recursive: true }); + } + writeFileSync(this.debugPromptsLogFile, '', 'utf-8'); + } } this.initialized = true; @@ -85,6 +102,7 @@ export class DebugLogger { reset(): void { this.debugEnabled = false; this.debugLogFile = null; + this.debugPromptsLogFile = null; this.initialized = false; this.verboseConsoleEnabled = false; } @@ -153,6 +171,19 @@ export class DebugLogger { } } + /** Write a prompt/response debug log entry */ + writePromptLog(record: PromptLogRecord): void { + if (!this.debugEnabled || !this.debugPromptsLogFile) { + return; + } + + try { + appendFileSync(this.debugPromptsLogFile, JSON.stringify(record) + '\n', 'utf-8'); + } catch { + // Silently fail - logging errors should not interrupt main flow + } + } + /** Create a scoped logger for a component */ createLogger(component: string) { return { @@ -203,6 +234,10 @@ export function errorLog(component: string, message: string, data?: unknown): vo DebugLogger.getInstance().writeLog('ERROR', component, message, data); } +export function writePromptLog(record: PromptLogRecord): void { + DebugLogger.getInstance().writePromptLog(record); +} + export function traceEnter(component: string, funcName: string, args?: Record): void { debugLog(component, `>> ${funcName}()`, args); } diff --git a/src/shared/utils/types.ts b/src/shared/utils/types.ts index 1968d58..3e4b24f 100644 --- a/src/shared/utils/types.ts +++ b/src/shared/utils/types.ts @@ -129,3 +129,13 @@ export interface LatestLogPointer { updatedAt: string; iterations: number; } + +/** Record for debug prompt/response log (debug-*-prompts.jsonl) */ +export interface PromptLogRecord { + movement: string; + phase: 1 | 2 | 3; + iteration: number; + prompt: string; + response: string; + timestamp: string; +} From 92f97bbd429cdb15e2897b2a1d7e165e4c197a03 Mon Sep 17 00:00:00 2001 From: nrs <38722970+nrslib@users.noreply.github.com> Date: Sun, 8 Feb 2026 07:21:25 +0900 Subject: [PATCH 3/3] takt: github-issue-105-aieejento-no (#138) --- src/__tests__/StreamDisplay.test.ts | 104 +++++++++++++++++++++++ src/__tests__/parallel-logger.test.ts | 84 ++++++++++++++++++ src/__tests__/strip-ansi.test.ts | 83 ++++++++++++++++++ src/core/piece/engine/parallel-logger.ts | 9 +- src/shared/ui/StreamDisplay.ts | 22 +++-- src/shared/utils/text.ts | 15 ++++ 6 files changed, 304 insertions(+), 13 deletions(-) create mode 100644 src/__tests__/strip-ansi.test.ts diff --git a/src/__tests__/StreamDisplay.test.ts b/src/__tests__/StreamDisplay.test.ts index 42bad7f..d0d7522 100644 --- a/src/__tests__/StreamDisplay.test.ts +++ b/src/__tests__/StreamDisplay.test.ts @@ -145,6 +145,110 @@ describe('StreamDisplay', () => { }); }); + describe('ANSI escape sequence stripping', () => { + it('should strip ANSI codes from text before writing to stdout', () => { + const display = new StreamDisplay('test-agent', false); + display.showText('\x1b[41mRed background\x1b[0m'); + + expect(stdoutWriteSpy).toHaveBeenCalledWith('Red background'); + }); + + it('should strip ANSI codes from thinking before writing to stdout', () => { + const display = new StreamDisplay('test-agent', false); + display.showThinking('\x1b[31mColored thinking\x1b[0m'); + + // chalk.gray.italic wraps the stripped text, so check it does NOT contain raw ANSI + const writtenText = stdoutWriteSpy.mock.calls[0]?.[0] as string; + expect(writtenText).not.toContain('\x1b[41m'); + expect(writtenText).not.toContain('\x1b[31m'); + expect(writtenText).toContain('Colored thinking'); + }); + + it('should accumulate stripped text in textBuffer', () => { + const display = new StreamDisplay('test-agent', false); + display.showText('\x1b[31mRed\x1b[0m'); + display.showText('\x1b[32m Green\x1b[0m'); + + // Flush should work correctly with stripped content + display.flushText(); + + // After flush, buffer is cleared — verify no crash and text was output + expect(stdoutWriteSpy).toHaveBeenCalledWith('Red'); + expect(stdoutWriteSpy).toHaveBeenCalledWith(' Green'); + }); + + it('should accumulate stripped text in thinkingBuffer', () => { + const display = new StreamDisplay('test-agent', false); + display.showThinking('\x1b[31mThought 1\x1b[0m'); + display.showThinking('\x1b[32m Thought 2\x1b[0m'); + + display.flushThinking(); + + // Verify stripped text was written (wrapped in chalk styling) + expect(stdoutWriteSpy).toHaveBeenCalledTimes(2); + }); + + it('should not strip ANSI from text that has no ANSI codes', () => { + const display = new StreamDisplay('test-agent', false); + display.showText('Plain text'); + + expect(stdoutWriteSpy).toHaveBeenCalledWith('Plain text'); + }); + + it('should strip ANSI codes from tool output before buffering', () => { + const display = new StreamDisplay('test-agent', false); + display.showToolUse('Bash', { command: 'ls' }); + display.showToolOutput('\x1b[32mgreen output\x1b[0m\n'); + + const outputLine = consoleLogSpy.mock.calls.find( + (call) => typeof call[0] === 'string' && (call[0] as string).includes('green output'), + ); + expect(outputLine).toBeDefined(); + expect(outputLine![0]).not.toContain('\x1b[32m'); + }); + + it('should strip ANSI codes from tool output across multiple chunks', () => { + const display = new StreamDisplay('test-agent', false); + display.showToolUse('Bash', { command: 'ls' }); + display.showToolOutput('\x1b[31mpartial'); + display.showToolOutput(' line\x1b[0m\n'); + + const outputLine = consoleLogSpy.mock.calls.find( + (call) => typeof call[0] === 'string' && (call[0] as string).includes('partial line'), + ); + expect(outputLine).toBeDefined(); + expect(outputLine![0]).not.toContain('\x1b[31m'); + }); + + it('should strip ANSI codes from tool result content', () => { + const display = new StreamDisplay('test-agent', false); + display.showToolUse('Read', { file_path: '/test.ts' }); + display.showToolResult('\x1b[41mResult with red bg\x1b[0m', false); + + const resultLine = consoleLogSpy.mock.calls.find( + (call) => typeof call[0] === 'string' && (call[0] as string).includes('✓'), + ); + expect(resultLine).toBeDefined(); + const fullOutput = resultLine!.join(' '); + expect(fullOutput).toContain('Result with red bg'); + expect(fullOutput).not.toContain('\x1b[41m'); + }); + + it('should strip ANSI codes from tool result error content', () => { + const display = new StreamDisplay('test-agent', false); + display.showToolUse('Bash', { command: 'fail' }); + display.showToolResult('\x1b[31mError message\x1b[0m', true); + + const errorLine = consoleLogSpy.mock.calls.find( + (call) => typeof call[0] === 'string' && (call[0] as string).includes('✗'), + ); + expect(errorLine).toBeDefined(); + const fullOutput = errorLine!.join(' '); + expect(fullOutput).toContain('Error message'); + expect(fullOutput).not.toContain('\x1b[31m'); + }); + }); + describe('progress prefix format', () => { it('should format progress as (iteration/max) step index/total', () => { const progressInfo: ProgressInfo = { diff --git a/src/__tests__/parallel-logger.test.ts b/src/__tests__/parallel-logger.test.ts index 5f41b50..36af15f 100644 --- a/src/__tests__/parallel-logger.test.ts +++ b/src/__tests__/parallel-logger.test.ts @@ -392,6 +392,90 @@ describe('ParallelLogger', () => { }); }); + describe('ANSI escape sequence stripping', () => { + it('should strip ANSI codes from text events', () => { + const logger = new ParallelLogger({ + subMovementNames: ['step-a'], + writeFn, + }); + const handler = logger.createStreamHandler('step-a', 0); + + handler({ type: 'text', data: { text: '\x1b[41mRed background\x1b[0m\n' } } as StreamEvent); + + expect(output).toHaveLength(1); + expect(output[0]).toContain('Red background'); + expect(output[0]).not.toContain('\x1b[41m'); + }); + + it('should strip ANSI codes from thinking events', () => { + const logger = new ParallelLogger({ + subMovementNames: ['step-a'], + writeFn, + }); + const handler = logger.createStreamHandler('step-a', 0); + + handler({ + type: 'thinking', + data: { thinking: '\x1b[31mColored thought\x1b[0m' }, + } as StreamEvent); + + expect(output).toHaveLength(1); + expect(output[0]).toContain('Colored thought'); + expect(output[0]).not.toContain('\x1b[31m'); + }); + + it('should strip ANSI codes from tool_output events', () => { + const logger = new ParallelLogger({ + subMovementNames: ['step-a'], + writeFn, + }); + const handler = logger.createStreamHandler('step-a', 0); + + handler({ + type: 'tool_output', + data: { tool: 'Bash', output: '\x1b[32mGreen output\x1b[0m' }, + } as StreamEvent); + + expect(output).toHaveLength(1); + expect(output[0]).toContain('Green output'); + expect(output[0]).not.toContain('\x1b[32m'); + }); + + it('should strip ANSI codes from tool_result events', () => { + const logger = new ParallelLogger({ + subMovementNames: ['step-a'], + writeFn, + }); + const handler = logger.createStreamHandler('step-a', 0); + + handler({ + type: 'tool_result', + data: { content: '\x1b[31mResult with ANSI\x1b[0m', isError: false }, + } as StreamEvent); + + expect(output).toHaveLength(1); + expect(output[0]).toContain('Result with ANSI'); + expect(output[0]).not.toContain('\x1b[31m'); + }); + + it('should strip ANSI codes from buffered text across multiple events', () => { + const logger = new ParallelLogger({ + subMovementNames: ['step-a'], + writeFn, + }); + const handler = logger.createStreamHandler('step-a', 0); + + handler({ type: 'text', data: { text: '\x1b[31mHello' } } as StreamEvent); + handler({ type: 'text', data: { text: ' World\x1b[0m\n' } } as StreamEvent); + + expect(output).toHaveLength(1); + expect(output[0]).toContain('Hello World'); + // The prefix contains its own ANSI codes (\x1b[36m, \x1b[0m), so + // verify the AI-originated \x1b[31m was stripped, not the prefix's codes + expect(output[0]).not.toContain('\x1b[31m'); + }); + }); + describe('interleaved output from multiple sub-movements', () => { it('should correctly interleave prefixed output', () => { const logger = new ParallelLogger({ diff --git a/src/__tests__/strip-ansi.test.ts b/src/__tests__/strip-ansi.test.ts new file mode 100644 index 0000000..7b7af1d --- /dev/null +++ b/src/__tests__/strip-ansi.test.ts @@ -0,0 +1,83 @@ +/** + * Tests for stripAnsi utility function + */ + +import { describe, it, expect } from 'vitest'; +import { stripAnsi } from '../shared/utils/text.js'; + +describe('stripAnsi', () => { + it('should return plain text unchanged', () => { + expect(stripAnsi('Hello World')).toBe('Hello World'); + }); + + it('should return empty string unchanged', () => { + expect(stripAnsi('')).toBe(''); + }); + + it('should strip foreground color codes', () => { + // Red text: ESC[31m ... ESC[0m + expect(stripAnsi('\x1b[31mError\x1b[0m')).toBe('Error'); + }); + + it('should strip background color codes', () => { + // Red background: ESC[41m ... ESC[0m + expect(stripAnsi('\x1b[41mHighlighted\x1b[0m')).toBe('Highlighted'); + }); + + it('should strip combined foreground and background codes', () => { + // White text on red background: ESC[37;41m + expect(stripAnsi('\x1b[37;41mAlert\x1b[0m')).toBe('Alert'); + }); + + it('should strip multiple SGR sequences in one string', () => { + const input = '\x1b[1mBold\x1b[0m normal \x1b[32mGreen\x1b[0m'; + expect(stripAnsi(input)).toBe('Bold normal Green'); + }); + + it('should strip 256-color sequences', () => { + // ESC[38;5;196m (foreground 256-color red) + expect(stripAnsi('\x1b[38;5;196mRed256\x1b[0m')).toBe('Red256'); + }); + + it('should strip cursor movement sequences', () => { + // Cursor up: ESC[1A, Cursor right: ESC[5C + expect(stripAnsi('\x1b[1AUp\x1b[5CRight')).toBe('UpRight'); + }); + + it('should strip erase sequences', () => { + // Clear line: ESC[2K + expect(stripAnsi('\x1b[2KCleared')).toBe('Cleared'); + }); + + it('should strip OSC sequences terminated by BEL', () => { + // Set terminal title: ESC]0;Title BEL + expect(stripAnsi('\x1b]0;My Title\x07Text')).toBe('Text'); + }); + + it('should strip OSC sequences terminated by ST', () => { + // Set terminal title: ESC]0;Title ESC\ + expect(stripAnsi('\x1b]0;My Title\x1b\\Text')).toBe('Text'); + }); + + it('should strip other single-character escape codes', () => { + // ESC followed by a single char (e.g., ESC M = reverse line feed) + expect(stripAnsi('\x1bMText')).toBe('Text'); + }); + + it('should preserve newlines and whitespace', () => { + expect(stripAnsi('\x1b[31mLine1\n\x1b[32mLine2\n')).toBe('Line1\nLine2\n'); + }); + + it('should strip sequences without a reset at the end', () => { + // Simulates the reported bug: background color set without reset + expect(stripAnsi('\x1b[41mRed background text')).toBe('Red background text'); + }); + + it('should handle text with only ANSI sequences', () => { + expect(stripAnsi('\x1b[31m\x1b[0m')).toBe(''); + }); + + it('should handle consecutive ANSI sequences', () => { + expect(stripAnsi('\x1b[1m\x1b[31m\x1b[42mStyled\x1b[0m')).toBe('Styled'); + }); +}); diff --git a/src/core/piece/engine/parallel-logger.ts b/src/core/piece/engine/parallel-logger.ts index 562d903..e3f2fd4 100644 --- a/src/core/piece/engine/parallel-logger.ts +++ b/src/core/piece/engine/parallel-logger.ts @@ -7,6 +7,7 @@ */ import type { StreamCallback, StreamEvent } from '../types.js'; +import { stripAnsi } from '../../../shared/utils/text.js'; /** ANSI color codes for sub-movement prefixes (cycled in order) */ const COLORS = ['\x1b[36m', '\x1b[33m', '\x1b[35m', '\x1b[32m'] as const; // cyan, yellow, magenta, green @@ -117,7 +118,7 @@ export class ParallelLogger { */ private handleTextEvent(name: string, prefix: string, text: string): void { const buffer = this.lineBuffers.get(name) ?? ''; - const combined = buffer + text; + const combined = buffer + stripAnsi(text); const parts = combined.split('\n'); // Last part is incomplete (no trailing newline) — keep in buffer @@ -145,13 +146,13 @@ export class ParallelLogger { text = `[tool] ${event.data.tool}`; break; case 'tool_result': - text = event.data.content; + text = stripAnsi(event.data.content); break; case 'tool_output': - text = event.data.output; + text = stripAnsi(event.data.output); break; case 'thinking': - text = event.data.thinking; + text = stripAnsi(event.data.thinking); break; default: return; diff --git a/src/shared/ui/StreamDisplay.ts b/src/shared/ui/StreamDisplay.ts index 53308be..581a79f 100644 --- a/src/shared/ui/StreamDisplay.ts +++ b/src/shared/ui/StreamDisplay.ts @@ -12,6 +12,7 @@ import chalk from 'chalk'; // dependent event-data types, which is out of scope for this refactoring. import type { StreamEvent, StreamCallback } from '../../core/piece/index.js'; import { truncate } from './LogManager.js'; +import { stripAnsi } from '../utils/text.js'; /** Progress information for stream display */ export interface ProgressInfo { @@ -116,7 +117,7 @@ export class StreamDisplay { this.lastToolUse = tool; } - this.toolOutputBuffer += output; + this.toolOutputBuffer += stripAnsi(output); const lines = this.toolOutputBuffer.split(/\r?\n/); this.toolOutputBuffer = lines.pop() ?? ''; @@ -129,11 +130,12 @@ export class StreamDisplay { showToolResult(content: string, isError: boolean): void { this.stopToolSpinner(); + const sanitizedContent = stripAnsi(content); if (this.quiet) { if (isError) { const toolName = this.lastToolUse || 'Tool'; - const errorContent = content || 'Unknown error'; + const errorContent = sanitizedContent || 'Unknown error'; console.log(chalk.red(` ✗ ${toolName}:`), chalk.red(truncate(errorContent, 70))); } this.lastToolUse = null; @@ -149,10 +151,10 @@ export class StreamDisplay { const toolName = this.lastToolUse || 'Tool'; if (isError) { - const errorContent = content || 'Unknown error'; + const errorContent = sanitizedContent || 'Unknown error'; console.log(chalk.red(` ✗ ${toolName}:`), chalk.red(truncate(errorContent, 70))); - } else if (content && content.length > 0) { - const preview = content.split('\n')[0] || content; + } else if (sanitizedContent && sanitizedContent.length > 0) { + const preview = sanitizedContent.split('\n')[0] || sanitizedContent; console.log(chalk.green(` ✓ ${toolName}`), chalk.gray(truncate(preview, 60))); } else { console.log(chalk.green(` ✓ ${toolName}`)); @@ -174,8 +176,9 @@ export class StreamDisplay { console.log(chalk.magenta(`💭 [${this.agentName}]${progressPart} thinking:`)); this.isFirstThinking = false; } - process.stdout.write(chalk.gray.italic(thinking)); - this.thinkingBuffer += thinking; + const sanitized = stripAnsi(thinking); + process.stdout.write(chalk.gray.italic(sanitized)); + this.thinkingBuffer += sanitized; } flushThinking(): void { @@ -200,8 +203,9 @@ export class StreamDisplay { console.log(chalk.cyan(`[${this.agentName}]${progressPart}:`)); this.isFirstText = false; } - process.stdout.write(text); - this.textBuffer += text; + const sanitized = stripAnsi(text); + process.stdout.write(sanitized); + this.textBuffer += sanitized; } flushText(): void { diff --git a/src/shared/utils/text.ts b/src/shared/utils/text.ts index e0e1bba..c49db62 100644 --- a/src/shared/utils/text.ts +++ b/src/shared/utils/text.ts @@ -35,6 +35,21 @@ export function getDisplayWidth(text: string): number { return width; } +// CSI (Control Sequence Introducer): ESC [ ... final_byte +// OSC (Operating System Command): ESC ] ... (ST | BEL) +// Other escape: ESC followed by a single character +// eslint-disable-next-line no-control-regex +const ANSI_PATTERN = /\x1b\[[0-9;]*[A-Za-z]|\x1b\][^\x07\x1b]*(?:\x07|\x1b\\)|\x1b[^[\]]/g; + +/** + * Strip all ANSI escape sequences from a string. + * Removes CSI sequences (colors, cursor movement, etc.), + * OSC sequences, and other single-character escape codes. + */ +export function stripAnsi(text: string): string { + return text.replace(ANSI_PATTERN, ''); +} + /** * Truncate plain text to fit within maxWidth display columns. * Appends '…' if truncated. The ellipsis itself counts as 1 column.