github-issue-245-report (#251)
* dist-tag 検証をリトライ付きに変更(npm レジストリの結果整合性対策) * takt run 実行時に蓋閉じスリープを抑制 * takt: github-issue-245-report
This commit is contained in:
parent
a82d6d9d8a
commit
39c587d67b
@ -32,6 +32,10 @@ function createContext(reportDir: string): PhaseRunnerContext {
|
||||
_sessionId,
|
||||
_overrides,
|
||||
) => ({ cwd: reportDir }),
|
||||
buildNewSessionReportOptions: (
|
||||
_step,
|
||||
_overrides,
|
||||
) => ({ cwd: reportDir }),
|
||||
updatePersonaSession: (_persona, sessionId) => {
|
||||
if (sessionId) {
|
||||
currentSessionId = sessionId;
|
||||
|
||||
211
src/__tests__/report-phase-retry.test.ts
Normal file
211
src/__tests__/report-phase-retry.test.ts
Normal file
@ -0,0 +1,211 @@
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
import { existsSync, mkdtempSync, readFileSync, 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: 'implement',
|
||||
persona: 'coder',
|
||||
personaDisplayName: 'Coder',
|
||||
instructionTemplate: 'Implement task',
|
||||
passPreviousResponse: false,
|
||||
outputContracts: [{ name: fileName }],
|
||||
};
|
||||
}
|
||||
|
||||
function createContext(reportDir: string, lastResponse = 'Phase 1 result'): PhaseRunnerContext {
|
||||
let currentSessionId = 'session-resume-1';
|
||||
|
||||
return {
|
||||
cwd: reportDir,
|
||||
reportDir,
|
||||
language: 'en',
|
||||
lastResponse,
|
||||
getSessionId: (_persona: string) => currentSessionId,
|
||||
buildResumeOptions: (_step, sessionId, overrides) => ({
|
||||
cwd: reportDir,
|
||||
sessionId,
|
||||
allowedTools: overrides.allowedTools,
|
||||
maxTurns: overrides.maxTurns,
|
||||
}),
|
||||
buildNewSessionReportOptions: (_step, overrides) => ({
|
||||
cwd: reportDir,
|
||||
allowedTools: overrides.allowedTools,
|
||||
maxTurns: overrides.maxTurns,
|
||||
}),
|
||||
updatePersonaSession: (_persona, sessionId) => {
|
||||
if (sessionId) {
|
||||
currentSessionId = sessionId;
|
||||
}
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
describe('runReportPhase retry with new session', () => {
|
||||
let tmpRoot: string;
|
||||
|
||||
beforeEach(() => {
|
||||
tmpRoot = mkdtempSync(join(tmpdir(), 'takt-report-retry-'));
|
||||
vi.resetAllMocks();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
if (existsSync(tmpRoot)) {
|
||||
rmSync(tmpRoot, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it('should retry with new session when first attempt returns empty content', async () => {
|
||||
// Given
|
||||
const reportDir = join(tmpRoot, '.takt', 'runs', 'sample-run', 'reports');
|
||||
const step = createStep('02-coder.md');
|
||||
const ctx = createContext(reportDir, 'Implemented feature X');
|
||||
const runAgentMock = vi.mocked(runAgent);
|
||||
runAgentMock
|
||||
.mockResolvedValueOnce({
|
||||
persona: 'coder',
|
||||
status: 'done',
|
||||
content: ' ',
|
||||
timestamp: new Date('2026-02-11T00:00:00Z'),
|
||||
sessionId: 'session-resume-2',
|
||||
})
|
||||
.mockResolvedValueOnce({
|
||||
persona: 'coder',
|
||||
status: 'done',
|
||||
content: '# Report\nRecovered output',
|
||||
timestamp: new Date('2026-02-11T00:00:01Z'),
|
||||
sessionId: 'session-fresh-1',
|
||||
});
|
||||
|
||||
// When
|
||||
await runReportPhase(step, 1, ctx);
|
||||
|
||||
// Then
|
||||
const reportPath = join(reportDir, '02-coder.md');
|
||||
expect(readFileSync(reportPath, 'utf-8')).toBe('# Report\nRecovered output');
|
||||
expect(runAgentMock).toHaveBeenCalledTimes(2);
|
||||
|
||||
const secondCallOptions = runAgentMock.mock.calls[1]?.[2] as { sessionId?: string };
|
||||
expect(secondCallOptions.sessionId).toBeUndefined();
|
||||
|
||||
const secondInstruction = runAgentMock.mock.calls[1]?.[1] as string;
|
||||
expect(secondInstruction).toContain('## Previous Work Context');
|
||||
expect(secondInstruction).toContain('Implemented feature X');
|
||||
});
|
||||
|
||||
it('should retry with new session when first attempt status is error', async () => {
|
||||
// Given
|
||||
const reportDir = join(tmpRoot, '.takt', 'runs', 'sample-run', 'reports');
|
||||
const step = createStep('03-review.md');
|
||||
const ctx = createContext(reportDir);
|
||||
const runAgentMock = vi.mocked(runAgent);
|
||||
runAgentMock
|
||||
.mockResolvedValueOnce({
|
||||
persona: 'coder',
|
||||
status: 'error',
|
||||
content: 'Tool use is not allowed in this phase',
|
||||
timestamp: new Date('2026-02-11T00:01:00Z'),
|
||||
error: 'Tool use is not allowed in this phase',
|
||||
})
|
||||
.mockResolvedValueOnce({
|
||||
persona: 'coder',
|
||||
status: 'done',
|
||||
content: 'Recovered report',
|
||||
timestamp: new Date('2026-02-11T00:01:01Z'),
|
||||
});
|
||||
|
||||
// When
|
||||
await runReportPhase(step, 1, ctx);
|
||||
|
||||
// Then
|
||||
const reportPath = join(reportDir, '03-review.md');
|
||||
expect(readFileSync(reportPath, 'utf-8')).toBe('Recovered report');
|
||||
expect(runAgentMock).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it('should throw when both attempts return empty output', async () => {
|
||||
// Given
|
||||
const reportDir = join(tmpRoot, '.takt', 'runs', 'sample-run', 'reports');
|
||||
const step = createStep('04-qa.md');
|
||||
const ctx = createContext(reportDir);
|
||||
const runAgentMock = vi.mocked(runAgent);
|
||||
runAgentMock
|
||||
.mockResolvedValueOnce({
|
||||
persona: 'coder',
|
||||
status: 'done',
|
||||
content: ' ',
|
||||
timestamp: new Date('2026-02-11T00:02:00Z'),
|
||||
})
|
||||
.mockResolvedValueOnce({
|
||||
persona: 'coder',
|
||||
status: 'done',
|
||||
content: '\n\n',
|
||||
timestamp: new Date('2026-02-11T00:02:01Z'),
|
||||
});
|
||||
|
||||
// When / Then
|
||||
await expect(runReportPhase(step, 1, ctx)).rejects.toThrow('Report phase failed for 04-qa.md: Report output is empty');
|
||||
expect(runAgentMock).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it('should not retry when first attempt succeeds', async () => {
|
||||
// Given
|
||||
const reportDir = join(tmpRoot, '.takt', 'runs', 'sample-run', 'reports');
|
||||
const step = createStep('05-ok.md');
|
||||
const ctx = createContext(reportDir);
|
||||
const runAgentMock = vi.mocked(runAgent);
|
||||
runAgentMock.mockResolvedValueOnce({
|
||||
persona: 'coder',
|
||||
status: 'done',
|
||||
content: 'Single-pass success',
|
||||
timestamp: new Date('2026-02-11T00:03:00Z'),
|
||||
sessionId: 'session-resume-2',
|
||||
});
|
||||
|
||||
// When
|
||||
await runReportPhase(step, 1, ctx);
|
||||
|
||||
// Then
|
||||
expect(runAgentMock).toHaveBeenCalledTimes(1);
|
||||
const reportPath = join(reportDir, '05-ok.md');
|
||||
expect(readFileSync(reportPath, 'utf-8')).toBe('Single-pass success');
|
||||
});
|
||||
|
||||
it('should return blocked result without retry', async () => {
|
||||
// Given
|
||||
const reportDir = join(tmpRoot, '.takt', 'runs', 'sample-run', 'reports');
|
||||
const step = createStep('06-blocked.md');
|
||||
const ctx = createContext(reportDir);
|
||||
const runAgentMock = vi.mocked(runAgent);
|
||||
runAgentMock.mockResolvedValueOnce({
|
||||
persona: 'coder',
|
||||
status: 'blocked',
|
||||
content: 'Need permission',
|
||||
timestamp: new Date('2026-02-11T00:04:00Z'),
|
||||
});
|
||||
|
||||
// When
|
||||
const result = await runReportPhase(step, 1, ctx);
|
||||
|
||||
// Then
|
||||
expect(result).toEqual({
|
||||
blocked: true,
|
||||
response: {
|
||||
persona: 'coder',
|
||||
status: 'blocked',
|
||||
content: 'Need permission',
|
||||
timestamp: new Date('2026-02-11T00:04:00Z'),
|
||||
},
|
||||
});
|
||||
expect(runAgentMock).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
@ -97,6 +97,19 @@ export class OptionsBuilder {
|
||||
};
|
||||
}
|
||||
|
||||
/** Build RunAgentOptions for Phase 2 retry with a new session */
|
||||
buildNewSessionReportOptions(
|
||||
step: PieceMovement,
|
||||
overrides: Pick<RunAgentOptions, 'allowedTools' | 'maxTurns'>,
|
||||
): RunAgentOptions {
|
||||
return {
|
||||
...this.buildBaseOptions(step),
|
||||
permissionMode: 'readonly',
|
||||
allowedTools: overrides.allowedTools,
|
||||
maxTurns: overrides.maxTurns,
|
||||
};
|
||||
}
|
||||
|
||||
/** Build PhaseRunnerContext for Phase 2/3 execution */
|
||||
buildPhaseRunnerContext(
|
||||
state: PieceState,
|
||||
@ -113,6 +126,7 @@ export class OptionsBuilder {
|
||||
lastResponse,
|
||||
getSessionId: (persona: string) => state.personaSessions.get(persona),
|
||||
buildResumeOptions: this.buildResumeOptions.bind(this),
|
||||
buildNewSessionReportOptions: this.buildNewSessionReportOptions.bind(this),
|
||||
updatePersonaSession,
|
||||
onPhaseStart,
|
||||
onPhaseComplete,
|
||||
|
||||
@ -25,6 +25,8 @@ export interface ReportInstructionContext {
|
||||
language?: Language;
|
||||
/** Target report file name (when generating a single report) */
|
||||
targetFile?: string;
|
||||
/** Last response from Phase 1 (used when report phase retries in a new session) */
|
||||
lastResponse?: string;
|
||||
}
|
||||
|
||||
/**
|
||||
@ -45,7 +47,6 @@ export class ReportInstructionBuilder {
|
||||
|
||||
const language = this.context.language ?? 'en';
|
||||
|
||||
// Build report context for Piece Context section
|
||||
let reportContext: string;
|
||||
if (this.context.targetFile) {
|
||||
reportContext = `- Report Directory: ${this.context.reportDir}/\n- Report File: ${this.context.reportDir}/${this.context.targetFile}`;
|
||||
@ -53,7 +54,6 @@ export class ReportInstructionBuilder {
|
||||
reportContext = renderReportContext(this.step.outputContracts, this.context.reportDir);
|
||||
}
|
||||
|
||||
// Build report output instruction
|
||||
let reportOutput = '';
|
||||
let hasReportOutput = false;
|
||||
const instrContext: InstructionContext = {
|
||||
@ -68,7 +68,6 @@ export class ReportInstructionBuilder {
|
||||
language,
|
||||
};
|
||||
|
||||
// Check for order instruction in first output contract item
|
||||
const firstContract = this.step.outputContracts[0];
|
||||
if (firstContract && isOutputContractItem(firstContract) && firstContract.order) {
|
||||
reportOutput = replaceTemplatePlaceholders(firstContract.order.trimEnd(), this.step, instrContext);
|
||||
@ -81,7 +80,6 @@ export class ReportInstructionBuilder {
|
||||
}
|
||||
}
|
||||
|
||||
// Build output contract (from first item's format)
|
||||
let outputContract = '';
|
||||
let hasOutputContract = false;
|
||||
if (firstContract && isOutputContractItem(firstContract) && firstContract.format) {
|
||||
@ -92,6 +90,8 @@ export class ReportInstructionBuilder {
|
||||
return loadTemplate('perform_phase2_message', language, {
|
||||
workingDirectory: this.context.cwd,
|
||||
reportContext,
|
||||
hasLastResponse: this.context.lastResponse != null && this.context.lastResponse.trim().length > 0,
|
||||
lastResponse: this.context.lastResponse ?? '',
|
||||
hasReportOutput,
|
||||
reportOutput,
|
||||
hasOutputContract,
|
||||
|
||||
@ -36,6 +36,8 @@ export interface PhaseRunnerContext {
|
||||
getSessionId: (persona: string) => string | undefined;
|
||||
/** Build resume options for a movement */
|
||||
buildResumeOptions: (step: PieceMovement, sessionId: string, overrides: Pick<RunAgentOptions, 'allowedTools' | 'maxTurns'>) => RunAgentOptions;
|
||||
/** Build options for report phase retry in a new session */
|
||||
buildNewSessionReportOptions: (step: PieceMovement, overrides: Pick<RunAgentOptions, 'allowedTools' | 'maxTurns'>) => RunAgentOptions;
|
||||
/** Update persona session after a phase run */
|
||||
updatePersonaSession: (persona: string, sessionId: string | undefined) => void;
|
||||
/** Callback for phase lifecycle logging */
|
||||
@ -140,52 +142,106 @@ export async function runReportPhase(
|
||||
targetFile: fileName,
|
||||
}).build();
|
||||
|
||||
ctx.onPhaseStart?.(step, 2, 'report', reportInstruction);
|
||||
|
||||
const reportOptions = ctx.buildResumeOptions(step, currentSessionId, {
|
||||
allowedTools: [],
|
||||
maxTurns: 3,
|
||||
});
|
||||
|
||||
let reportResponse;
|
||||
try {
|
||||
reportResponse = await runAgent(step.persona, reportInstruction, reportOptions);
|
||||
} catch (error) {
|
||||
const errorMsg = error instanceof Error ? error.message : String(error);
|
||||
ctx.onPhaseComplete?.(step, 2, 'report', '', 'error', errorMsg);
|
||||
throw error;
|
||||
const firstAttempt = await runSingleReportAttempt(step, reportInstruction, reportOptions, ctx);
|
||||
if (firstAttempt.kind === 'blocked') {
|
||||
return { blocked: true, response: firstAttempt.response };
|
||||
}
|
||||
if (firstAttempt.kind === 'success') {
|
||||
writeReportFile(ctx.reportDir, fileName, firstAttempt.content);
|
||||
if (firstAttempt.response.sessionId) {
|
||||
currentSessionId = firstAttempt.response.sessionId;
|
||||
ctx.updatePersonaSession(sessionKey, currentSessionId);
|
||||
}
|
||||
log.debug('Report file generated', { movement: step.name, fileName });
|
||||
continue;
|
||||
}
|
||||
|
||||
if (reportResponse.status === 'blocked') {
|
||||
ctx.onPhaseComplete?.(step, 2, 'report', reportResponse.content, reportResponse.status);
|
||||
return { blocked: true, response: reportResponse };
|
||||
log.info('Report phase failed, retrying with new session', {
|
||||
movement: step.name,
|
||||
fileName,
|
||||
reason: firstAttempt.errorMessage,
|
||||
});
|
||||
|
||||
const retryInstruction = new ReportInstructionBuilder(step, {
|
||||
cwd: ctx.cwd,
|
||||
reportDir: ctx.reportDir,
|
||||
movementIteration: movementIteration,
|
||||
language: ctx.language,
|
||||
targetFile: fileName,
|
||||
lastResponse: ctx.lastResponse,
|
||||
}).build();
|
||||
const retryOptions = ctx.buildNewSessionReportOptions(step, {
|
||||
allowedTools: [],
|
||||
maxTurns: 3,
|
||||
});
|
||||
|
||||
const retryAttempt = await runSingleReportAttempt(step, retryInstruction, retryOptions, ctx);
|
||||
if (retryAttempt.kind === 'blocked') {
|
||||
return { blocked: true, response: retryAttempt.response };
|
||||
}
|
||||
if (retryAttempt.kind === 'retryable_failure') {
|
||||
throw new Error(`Report phase failed for ${fileName}: ${retryAttempt.errorMessage}`);
|
||||
}
|
||||
|
||||
if (reportResponse.status !== 'done') {
|
||||
const errorMsg = reportResponse.error || reportResponse.content || 'Unknown error';
|
||||
ctx.onPhaseComplete?.(step, 2, 'report', reportResponse.content, reportResponse.status, errorMsg);
|
||||
throw new Error(`Report phase failed for ${fileName}: ${errorMsg}`);
|
||||
}
|
||||
|
||||
const content = reportResponse.content.trim();
|
||||
if (content.length === 0) {
|
||||
throw new Error(`Report output is empty for file: ${fileName}`);
|
||||
}
|
||||
|
||||
writeReportFile(ctx.reportDir, fileName, content);
|
||||
|
||||
if (reportResponse.sessionId) {
|
||||
currentSessionId = reportResponse.sessionId;
|
||||
writeReportFile(ctx.reportDir, fileName, retryAttempt.content);
|
||||
if (retryAttempt.response.sessionId) {
|
||||
currentSessionId = retryAttempt.response.sessionId;
|
||||
ctx.updatePersonaSession(sessionKey, currentSessionId);
|
||||
}
|
||||
|
||||
ctx.onPhaseComplete?.(step, 2, 'report', reportResponse.content, reportResponse.status);
|
||||
log.debug('Report file generated', { movement: step.name, fileName });
|
||||
}
|
||||
|
||||
log.debug('Report phase complete', { movement: step.name, filesGenerated: reportFiles.length });
|
||||
}
|
||||
|
||||
type ReportAttemptResult =
|
||||
| { kind: 'success'; content: string; response: AgentResponse }
|
||||
| { kind: 'blocked'; response: AgentResponse }
|
||||
| { kind: 'retryable_failure'; errorMessage: string };
|
||||
|
||||
async function runSingleReportAttempt(
|
||||
step: PieceMovement,
|
||||
instruction: string,
|
||||
options: RunAgentOptions,
|
||||
ctx: PhaseRunnerContext,
|
||||
): Promise<ReportAttemptResult> {
|
||||
ctx.onPhaseStart?.(step, 2, 'report', instruction);
|
||||
|
||||
let response: AgentResponse;
|
||||
try {
|
||||
response = await runAgent(step.persona, instruction, options);
|
||||
} catch (error) {
|
||||
const errorMsg = error instanceof Error ? error.message : String(error);
|
||||
ctx.onPhaseComplete?.(step, 2, 'report', '', 'error', errorMsg);
|
||||
throw error;
|
||||
}
|
||||
|
||||
if (response.status === 'blocked') {
|
||||
ctx.onPhaseComplete?.(step, 2, 'report', response.content, response.status);
|
||||
return { kind: 'blocked', response };
|
||||
}
|
||||
|
||||
if (response.status !== 'done') {
|
||||
const errorMessage = response.error || response.content || 'Unknown error';
|
||||
ctx.onPhaseComplete?.(step, 2, 'report', response.content, response.status, errorMessage);
|
||||
return { kind: 'retryable_failure', errorMessage };
|
||||
}
|
||||
|
||||
const trimmedContent = response.content.trim();
|
||||
if (trimmedContent.length === 0) {
|
||||
const errorMessage = 'Report output is empty';
|
||||
ctx.onPhaseComplete?.(step, 2, 'report', response.content, 'error', errorMessage);
|
||||
return { kind: 'retryable_failure', errorMessage };
|
||||
}
|
||||
|
||||
ctx.onPhaseComplete?.(step, 2, 'report', response.content, response.status);
|
||||
return { kind: 'success', content: trimmedContent, response };
|
||||
}
|
||||
|
||||
/**
|
||||
* Phase 3: Status judgment.
|
||||
* Uses the 'conductor' agent in a new session to output a status tag.
|
||||
@ -198,7 +254,6 @@ export async function runStatusJudgmentPhase(
|
||||
): Promise<string> {
|
||||
log.debug('Running status judgment phase', { movement: step.name });
|
||||
|
||||
// フォールバック戦略を順次試行(AutoSelectStrategy含む)
|
||||
const strategies = JudgmentStrategyFactory.createStrategies();
|
||||
const sessionKey = buildSessionKey(step);
|
||||
const judgmentContext: JudgmentContext = {
|
||||
@ -234,7 +289,6 @@ export async function runStatusJudgmentPhase(
|
||||
}
|
||||
}
|
||||
|
||||
// 全戦略失敗
|
||||
const errorMsg = 'All judgment strategies failed';
|
||||
ctx.onPhaseComplete?.(step, 3, 'judge', '', 'error', errorMsg);
|
||||
throw new Error(errorMsg);
|
||||
|
||||
@ -1,7 +1,7 @@
|
||||
<!--
|
||||
template: perform_phase2_message
|
||||
phase: 2 (report output)
|
||||
vars: workingDirectory, reportContext, hasReportOutput, reportOutput,
|
||||
vars: workingDirectory, reportContext, hasLastResponse, lastResponse, hasReportOutput, reportOutput,
|
||||
hasOutputContract, outputContract
|
||||
builder: ReportInstructionBuilder
|
||||
-->
|
||||
@ -17,6 +17,13 @@ Note: This section is metadata. Follow the language used in the rest of the prom
|
||||
|
||||
## Piece Context
|
||||
{{reportContext}}
|
||||
{{#if hasLastResponse}}
|
||||
|
||||
## Previous Work Context
|
||||
The following is the output from Phase 1 (your main work). Use this as context to generate the report:
|
||||
|
||||
{{lastResponse}}
|
||||
{{/if}}
|
||||
|
||||
## Instructions
|
||||
Respond with the results of the work you just completed as a report. **Tools are not available in this phase. Respond with the report content directly as text.**
|
||||
|
||||
@ -1,7 +1,7 @@
|
||||
<!--
|
||||
template: perform_phase2_message
|
||||
phase: 2 (report output)
|
||||
vars: workingDirectory, reportContext, hasReportOutput, reportOutput,
|
||||
vars: workingDirectory, reportContext, hasLastResponse, lastResponse, hasReportOutput, reportOutput,
|
||||
hasOutputContract, outputContract
|
||||
builder: ReportInstructionBuilder
|
||||
-->
|
||||
@ -16,6 +16,13 @@
|
||||
|
||||
## Piece Context
|
||||
{{reportContext}}
|
||||
{{#if hasLastResponse}}
|
||||
|
||||
## Previous Work Context
|
||||
以下はPhase 1(本来の作業)の出力です。レポート生成の文脈として使用してください:
|
||||
|
||||
{{lastResponse}}
|
||||
{{/if}}
|
||||
|
||||
## Instructions
|
||||
あなたが今行った作業の結果をレポートとして回答してください。**このフェーズではツールは使えません。レポート内容をテキストとして直接回答してください。**
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user