diff --git a/CLAUDE.md b/CLAUDE.md index d695cf4..c4b2a60 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -406,7 +406,7 @@ Key constraints: - **Ephemeral lifecycle**: Clone is created → task runs → auto-commit + push → clone is deleted. Branches are the single source of truth. - **Session isolation**: Claude Code sessions are stored per-cwd in `~/.claude/projects/{encoded-path}/`. Sessions from the main project cannot be resumed in a clone. The engine skips session resume when `cwd !== projectCwd`. - **No node_modules**: Clones only contain tracked files. `node_modules/` is absent. -- **Dual cwd**: `cwd` = clone path (where agents run), `projectCwd` = project root. Reports write to `cwd/.takt/reports/` (clone) to prevent agents from discovering the main repository. Logs and session data write to `projectCwd`. +- **Dual cwd**: `cwd` = clone path (where agents run), `projectCwd` = project root. Reports write to `cwd/.takt/runs/{slug}/reports/` (clone) to prevent agents from discovering the main repository. Logs and session data write to `projectCwd`. - **List**: Use `takt list` to list branches. Instruct action creates a temporary clone for the branch, executes, pushes, then removes the clone. ## Error Propagation @@ -455,10 +455,10 @@ Debug logs are written to `.takt/logs/debug.log` (ndjson format). Log levels: `d - If persona file doesn't exist, the persona string is used as inline system prompt **Report directory structure:** -- Report dirs are created at `.takt/reports/{timestamp}-{slug}/` +- Report dirs are created at `.takt/runs/{timestamp}-{slug}/reports/` - Report files specified in `step.report` are written relative to report dir - Report dir path is available as `{report_dir}` variable in instruction templates -- When `cwd !== projectCwd` (worktree execution), reports write to `cwd/.takt/reports/` (clone dir) to prevent agents from discovering the main repository path +- When `cwd !== projectCwd` (worktree execution), reports write to `cwd/.takt/runs/{slug}/reports/` (clone dir) to prevent agents from discovering the main repository path **Session continuity across phases:** - Agent sessions persist across Phase 1 → Phase 2 → Phase 3 for context continuity @@ -470,7 +470,7 @@ Debug logs are written to `.takt/logs/debug.log` (ndjson format). Log levels: `d - `git clone --shared` creates independent `.git` directory (not `git worktree`) - Clone cwd ≠ project cwd: agents work in clone, reports write to clone, logs write to project - Session resume is skipped when `cwd !== projectCwd` to avoid cross-directory contamination -- Reports write to `cwd/.takt/reports/` (clone) to prevent agents from discovering the main repository path via instruction +- Reports write to `cwd/.takt/runs/{slug}/reports/` (clone) to prevent agents from discovering the main repository path via instruction - Clones are ephemeral: created → task runs → auto-commit + push → deleted - Use `takt list` to manage task branches after clone deletion diff --git a/src/__tests__/engine-test-helpers.test.ts b/src/__tests__/engine-test-helpers.test.ts new file mode 100644 index 0000000..f917be1 --- /dev/null +++ b/src/__tests__/engine-test-helpers.test.ts @@ -0,0 +1,21 @@ +import { describe, it, expect, vi } from 'vitest'; + +import { cleanupPieceEngine } from './engine-test-helpers.js'; + +describe('cleanupPieceEngine', () => { + it('should remove all listeners when engine has removeAllListeners function', () => { + const removeAllListeners = vi.fn(); + const engine = { removeAllListeners }; + + cleanupPieceEngine(engine); + + expect(removeAllListeners).toHaveBeenCalledOnce(); + }); + + it('should not throw when engine does not have removeAllListeners function', () => { + expect(() => cleanupPieceEngine({})).not.toThrow(); + expect(() => cleanupPieceEngine(null)).not.toThrow(); + expect(() => cleanupPieceEngine(undefined)).not.toThrow(); + expect(() => cleanupPieceEngine({ removeAllListeners: 'no-op' })).not.toThrow(); + }); +}); diff --git a/src/__tests__/engine-test-helpers.ts b/src/__tests__/engine-test-helpers.ts index d438c11..e658a5d 100644 --- a/src/__tests__/engine-test-helpers.ts +++ b/src/__tests__/engine-test-helpers.ts @@ -178,25 +178,27 @@ export function applyDefaultMocks(): void { vi.mocked(generateReportDir).mockReturnValue('test-report-dir'); } +type RemovableListeners = { + removeAllListeners: () => void; +}; + +function hasRemovableListeners(value: unknown): value is RemovableListeners { + if (!value || typeof value !== 'object') { + return false; + } + if (!('removeAllListeners' in value)) { + return false; + } + const candidate = value as { removeAllListeners: unknown }; + return typeof candidate.removeAllListeners === 'function'; +} + /** * Clean up PieceEngine instances to prevent EventEmitter memory leaks. * Call this in afterEach to ensure all event listeners are removed. */ -type ListenerCleanupTarget = { - removeAllListeners: () => void; -}; - -function isListenerCleanupTarget(value: unknown): value is ListenerCleanupTarget { - return ( - typeof value === 'object' && - value !== null && - 'removeAllListeners' in value && - typeof value.removeAllListeners === 'function' - ); -} - export function cleanupPieceEngine(engine: unknown): void { - if (isListenerCleanupTarget(engine)) { + if (hasRemovableListeners(engine)) { engine.removeAllListeners(); } } diff --git a/src/__tests__/instruction-helpers.test.ts b/src/__tests__/instruction-helpers.test.ts index 0b9adff..cec104f 100644 --- a/src/__tests__/instruction-helpers.test.ts +++ b/src/__tests__/instruction-helpers.test.ts @@ -101,7 +101,7 @@ describe('renderReportOutputInstruction', () => { const result = renderReportOutputInstruction(step, ctx, 'en'); expect(result).toContain('Report output'); expect(result).toContain('Report File'); - expect(result).toContain('Iteration 2'); + expect(result).toContain('Move current content to `logs/reports-history/`'); }); it('should render English multi-file instruction', () => { @@ -121,6 +121,7 @@ describe('renderReportOutputInstruction', () => { const result = renderReportOutputInstruction(step, ctx, 'ja'); expect(result).toContain('レポート出力'); expect(result).toContain('Report File'); + expect(result).toContain('`logs/reports-history/`'); }); it('should render Japanese multi-file instruction', () => { diff --git a/src/__tests__/instructionBuilder.test.ts b/src/__tests__/instructionBuilder.test.ts index e78a868..d0b81d0 100644 --- a/src/__tests__/instructionBuilder.test.ts +++ b/src/__tests__/instructionBuilder.test.ts @@ -802,7 +802,7 @@ describe('instruction-builder', () => { expect(result).toContain('**Report output:** Output to the `Report File` specified above.'); expect(result).toContain('- If file does not exist: Create new file'); - expect(result).toContain('Append with `## Iteration 1` section'); + expect(result).toContain('- If file exists: Move current content to `logs/reports-history/` and overwrite with latest report'); }); it('should include explicit order instead of auto-generated', () => { @@ -833,14 +833,14 @@ describe('instruction-builder', () => { expect(result).toContain('# Plan'); }); - it('should replace {movement_iteration} in report output instruction', () => { + it('should include overwrite-and-archive rule in report output instruction', () => { const step = createMinimalStep('Do work'); step.outputContracts = [{ name: '00-plan.md' }]; const ctx = createReportContext({ movementIteration: 5 }); const result = buildReportInstruction(step, ctx); - expect(result).toContain('Append with `## Iteration 5` section'); + expect(result).toContain('Move current content to `logs/reports-history/` and overwrite with latest report'); }); it('should include instruction body text', () => { diff --git a/src/__tests__/judgment-fallback.test.ts b/src/__tests__/judgment-fallback.test.ts index e7d80bf..0d7d560 100644 --- a/src/__tests__/judgment-fallback.test.ts +++ b/src/__tests__/judgment-fallback.test.ts @@ -3,8 +3,12 @@ */ import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { mkdtempSync, mkdirSync, rmSync, writeFileSync } from 'node:fs'; +import { join } from 'node:path'; +import { tmpdir } from 'node:os'; import type { PieceMovement } from '../core/models/types.js'; import type { JudgmentContext } from '../core/piece/judgment/FallbackStrategy.js'; +import { runAgent } from '../agents/runner.js'; import { AutoSelectStrategy, ReportBasedStrategy, @@ -88,6 +92,48 @@ describe('JudgmentStrategies', () => { // mockStep has no outputContracts field → getReportFiles returns [] expect(strategy.canApply(mockContext)).toBe(false); }); + + it('should use only latest report file from reports directory', async () => { + const tmpRoot = mkdtempSync(join(tmpdir(), 'takt-judgment-report-')); + try { + const reportDir = join(tmpRoot, '.takt', 'runs', 'sample-run', 'reports'); + const historyDir = join(tmpRoot, '.takt', 'runs', 'sample-run', 'logs', 'reports-history'); + mkdirSync(reportDir, { recursive: true }); + mkdirSync(historyDir, { recursive: true }); + + const latestFile = '05-architect-review.md'; + writeFileSync(join(reportDir, latestFile), 'LATEST-ONLY-CONTENT'); + writeFileSync(join(historyDir, '05-architect-review.20260210T061143Z.md'), 'OLD-HISTORY-CONTENT'); + + const stepWithOutputContracts: PieceMovement = { + ...mockStep, + outputContracts: [{ name: latestFile }], + }; + + const runAgentMock = vi.mocked(runAgent); + runAgentMock.mockResolvedValue({ + persona: 'conductor', + status: 'done', + content: '[TEST-MOVEMENT:1]', + timestamp: new Date('2026-02-10T07:11:43Z'), + }); + + const strategy = new ReportBasedStrategy(); + const result = await strategy.execute({ + ...mockContext, + step: stepWithOutputContracts, + reportDir, + }); + + expect(result.success).toBe(true); + expect(runAgentMock).toHaveBeenCalledTimes(1); + const instruction = runAgentMock.mock.calls[0]?.[1]; + expect(instruction).toContain('LATEST-ONLY-CONTENT'); + expect(instruction).not.toContain('OLD-HISTORY-CONTENT'); + } finally { + rmSync(tmpRoot, { recursive: true, force: true }); + } + }); }); describe('ResponseBasedStrategy', () => { diff --git a/src/__tests__/phase-runner-report-history.test.ts b/src/__tests__/phase-runner-report-history.test.ts new file mode 100644 index 0000000..f296066 --- /dev/null +++ b/src/__tests__/phase-runner-report-history.test.ts @@ -0,0 +1,143 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { existsSync, mkdtempSync, readFileSync, readdirSync, rmSync } from 'node:fs'; +import { join } from 'node:path'; +import { tmpdir } from 'node:os'; +import { runReportPhase, type PhaseRunnerContext } from '../core/piece/phase-runner.js'; +import type { PieceMovement } from '../core/models/types.js'; + +vi.mock('../agents/runner.js', () => ({ + runAgent: vi.fn(), +})); + +import { runAgent } from '../agents/runner.js'; + +function createStep(fileName: string): PieceMovement { + return { + name: 'reviewers', + personaDisplayName: 'Reviewers', + instructionTemplate: 'review', + passPreviousResponse: false, + outputContracts: [{ name: fileName }], + }; +} + +function createContext(reportDir: string): PhaseRunnerContext { + let currentSessionId = 'session-1'; + return { + cwd: reportDir, + reportDir, + getSessionId: (_persona: string) => currentSessionId, + buildResumeOptions: ( + _step, + _sessionId, + _overrides, + ) => ({ cwd: reportDir }), + updatePersonaSession: (_persona, sessionId) => { + if (sessionId) { + currentSessionId = sessionId; + } + }, + }; +} + +describe('runReportPhase report history behavior', () => { + let tmpRoot: string; + + beforeEach(() => { + tmpRoot = mkdtempSync(join(tmpdir(), 'takt-report-history-')); + vi.resetAllMocks(); + }); + + afterEach(() => { + vi.useRealTimers(); + if (existsSync(tmpRoot)) { + rmSync(tmpRoot, { recursive: true, force: true }); + } + }); + + it('should overwrite report file and archive previous content to reports-history', async () => { + // Given + const reportDir = join(tmpRoot, '.takt', 'runs', 'sample-run', 'reports'); + const step = createStep('05-architect-review.md'); + const ctx = createContext(reportDir); + const runAgentMock = vi.mocked(runAgent); + runAgentMock + .mockResolvedValueOnce({ + persona: 'reviewers', + status: 'done', + content: 'First review result', + timestamp: new Date('2026-02-10T06:11:43Z'), + sessionId: 'session-2', + }) + .mockResolvedValueOnce({ + persona: 'reviewers', + status: 'done', + content: 'Second review result', + timestamp: new Date('2026-02-10T06:14:37Z'), + sessionId: 'session-3', + }); + + // When + await runReportPhase(step, 1, ctx); + await runReportPhase(step, 2, ctx); + + // Then + const latestPath = join(reportDir, '05-architect-review.md'); + const latestContent = readFileSync(latestPath, 'utf-8'); + expect(latestContent).toBe('Second review result'); + + const historyDir = join(tmpRoot, '.takt', 'runs', 'sample-run', 'logs', 'reports-history'); + const historyFiles = readdirSync(historyDir); + expect(historyFiles).toHaveLength(1); + expect(historyFiles[0]).toMatch(/^05-architect-review\.\d{8}T\d{6}Z\.md$/); + + const archivedContent = readFileSync(join(historyDir, historyFiles[0]!), 'utf-8'); + expect(archivedContent).toBe('First review result'); + }); + + it('should add sequence suffix when history file name collides in the same second', async () => { + // Given + vi.useFakeTimers(); + vi.setSystemTime(new Date('2026-02-10T06:11:43Z')); + + const reportDir = join(tmpRoot, '.takt', 'runs', 'sample-run', 'reports'); + const step = createStep('06-qa-review.md'); + const ctx = createContext(reportDir); + const runAgentMock = vi.mocked(runAgent); + runAgentMock + .mockResolvedValueOnce({ + persona: 'reviewers', + status: 'done', + content: 'v1', + timestamp: new Date('2026-02-10T06:11:43Z'), + sessionId: 'session-2', + }) + .mockResolvedValueOnce({ + persona: 'reviewers', + status: 'done', + content: 'v2', + timestamp: new Date('2026-02-10T06:11:43Z'), + sessionId: 'session-3', + }) + .mockResolvedValueOnce({ + persona: 'reviewers', + status: 'done', + content: 'v3', + timestamp: new Date('2026-02-10T06:11:43Z'), + sessionId: 'session-4', + }); + + // When + await runReportPhase(step, 1, ctx); + await runReportPhase(step, 2, ctx); + await runReportPhase(step, 3, ctx); + + // Then + const historyDir = join(tmpRoot, '.takt', 'runs', 'sample-run', 'logs', 'reports-history'); + const historyFiles = readdirSync(historyDir).sort(); + expect(historyFiles).toEqual([ + '06-qa-review.20260210T061143Z.1.md', + '06-qa-review.20260210T061143Z.md', + ]); + }); +}); diff --git a/src/core/piece/instruction/InstructionBuilder.ts b/src/core/piece/instruction/InstructionBuilder.ts index 27ce376..30b0fb5 100644 --- a/src/core/piece/instruction/InstructionBuilder.ts +++ b/src/core/piece/instruction/InstructionBuilder.ts @@ -298,21 +298,21 @@ export function renderReportOutputInstruction( let heading: string; let createRule: string; - let appendRule: string; + let overwriteRule: string; if (language === 'ja') { heading = isMulti ? '**レポート出力:** Report Files に出力してください。' : '**レポート出力:** `Report File` に出力してください。'; createRule = '- ファイルが存在しない場合: 新規作成'; - appendRule = `- ファイルが存在する場合: \`## Iteration ${context.movementIteration}\` セクションを追記`; + overwriteRule = '- ファイルが存在する場合: 既存内容を `logs/reports-history/` に退避し、最新内容で上書き'; } else { heading = isMulti ? '**Report output:** Output to the `Report Files` specified above.' : '**Report output:** Output to the `Report File` specified above.'; createRule = '- If file does not exist: Create new file'; - appendRule = `- If file exists: Append with \`## Iteration ${context.movementIteration}\` section`; + overwriteRule = '- If file exists: Move current content to `logs/reports-history/` and overwrite with latest report'; } - return `${heading}\n${createRule}\n${appendRule}`; + return `${heading}\n${createRule}\n${overwriteRule}`; } diff --git a/src/core/piece/phase-runner.ts b/src/core/piece/phase-runner.ts index 21b2e62..ac784b5 100644 --- a/src/core/piece/phase-runner.ts +++ b/src/core/piece/phase-runner.ts @@ -5,8 +5,8 @@ * as session-resume operations. */ -import { appendFileSync, existsSync, mkdirSync, writeFileSync } from 'node:fs'; -import { dirname, resolve, sep } from 'node:path'; +import { existsSync, mkdirSync, readFileSync, writeFileSync } from 'node:fs'; +import { dirname, parse, resolve, sep } from 'node:path'; import type { PieceMovement, Language } from '../models/types.js'; import type { PhaseName } from './types.js'; import { runAgent, type RunAgentOptions } from '../../agents/runner.js'; @@ -49,6 +49,41 @@ export function needsStatusJudgmentPhase(step: PieceMovement): boolean { return hasTagBasedRules(step); } +function formatHistoryTimestamp(date: Date): string { + const year = date.getUTCFullYear(); + const month = String(date.getUTCMonth() + 1).padStart(2, '0'); + const day = String(date.getUTCDate()).padStart(2, '0'); + const hour = String(date.getUTCHours()).padStart(2, '0'); + const minute = String(date.getUTCMinutes()).padStart(2, '0'); + const second = String(date.getUTCSeconds()).padStart(2, '0'); + return `${year}${month}${day}T${hour}${minute}${second}Z`; +} + +function buildHistoryFileName(fileName: string, timestamp: string, sequence: number): string { + const parsed = parse(fileName); + const duplicateSuffix = sequence === 0 ? '' : `.${sequence}`; + return `${parsed.name}.${timestamp}${duplicateSuffix}${parsed.ext}`; +} + +function backupExistingReport(reportDir: string, fileName: string, targetPath: string): void { + if (!existsSync(targetPath)) { + return; + } + + const currentContent = readFileSync(targetPath, 'utf-8'); + const historyDir = resolve(reportDir, '..', 'logs', 'reports-history'); + mkdirSync(historyDir, { recursive: true }); + + const timestamp = formatHistoryTimestamp(new Date()); + let sequence = 0; + let historyPath = resolve(historyDir, buildHistoryFileName(fileName, timestamp, sequence)); + while (existsSync(historyPath)) { + sequence += 1; + historyPath = resolve(historyDir, buildHistoryFileName(fileName, timestamp, sequence)); + } + + writeFileSync(historyPath, currentContent); +} function writeReportFile(reportDir: string, fileName: string, content: string): void { const baseDir = resolve(reportDir); @@ -58,11 +93,8 @@ function writeReportFile(reportDir: string, fileName: string, content: string): throw new Error(`Report file path escapes report directory: ${fileName}`); } mkdirSync(dirname(targetPath), { recursive: true }); - if (existsSync(targetPath)) { - appendFileSync(targetPath, `\n\n${content}`); - } else { - writeFileSync(targetPath, content); - } + backupExistingReport(baseDir, fileName, targetPath); + writeFileSync(targetPath, content); } /**