diff --git a/src/__tests__/report-phase-blocked.test.ts b/src/__tests__/report-phase-blocked.test.ts new file mode 100644 index 0000000..3afad14 --- /dev/null +++ b/src/__tests__/report-phase-blocked.test.ts @@ -0,0 +1,224 @@ +/** + * PieceEngine integration tests: Report phase (Phase 2) blocked handling. + * + * Covers: + * - Report phase blocked propagates to PieceEngine's handleBlocked flow + * - User input triggers full movement retry (Phase 1 → 2 → 3) + * - Null user input aborts the piece + */ + +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { existsSync, rmSync } from 'node:fs'; + +// --- Mock setup (must be before imports that use these modules) --- + +vi.mock('../agents/runner.js', () => ({ + runAgent: vi.fn(), +})); + +vi.mock('../core/piece/evaluation/index.js', () => ({ + detectMatchedRule: vi.fn(), +})); + +vi.mock('../core/piece/phase-runner.js', () => ({ + needsStatusJudgmentPhase: vi.fn().mockReturnValue(false), + runReportPhase: vi.fn().mockResolvedValue(undefined), + runStatusJudgmentPhase: vi.fn().mockResolvedValue(''), +})); + +vi.mock('../shared/utils/index.js', async (importOriginal) => ({ + ...(await importOriginal>()), + generateReportDir: vi.fn().mockReturnValue('test-report-dir'), +})); + +// --- Imports (after mocks) --- + +import { PieceEngine } from '../core/piece/index.js'; +import { runReportPhase } from '../core/piece/index.js'; +import { + makeResponse, + makeMovement, + buildDefaultPieceConfig, + mockRunAgentSequence, + mockDetectMatchedRuleSequence, + createTestTmpDir, + applyDefaultMocks, +} from './engine-test-helpers.js'; +import type { PieceConfig, OutputContractItem } from '../core/models/index.js'; + +/** + * Build a piece config where a movement has outputContracts (triggering report phase). + * plan → implement (with report) → supervise + */ +function buildConfigWithReport(): PieceConfig { + const reportContract: OutputContractItem = { + name: '02-coder-scope.md', + label: 'Scope', + description: 'Scope report', + }; + + return buildDefaultPieceConfig({ + movements: [ + makeMovement('plan', { + rules: [ + { condition: 'Requirements are clear', next: 'implement' }, + { condition: 'Requirements unclear', next: 'ABORT' }, + ], + }), + makeMovement('implement', { + outputContracts: [reportContract], + rules: [ + { condition: 'Implementation complete', next: 'supervise' }, + { condition: 'Cannot proceed', next: 'plan' }, + ], + }), + makeMovement('supervise', { + rules: [ + { condition: 'All checks passed', next: 'COMPLETE' }, + { condition: 'Requirements unmet', next: 'plan' }, + ], + }), + ], + }); +} + +describe('PieceEngine Integration: Report Phase Blocked Handling', () => { + let tmpDir: string; + + beforeEach(() => { + vi.resetAllMocks(); + applyDefaultMocks(); + tmpDir = createTestTmpDir(); + }); + + afterEach(() => { + if (existsSync(tmpDir)) { + rmSync(tmpDir, { recursive: true, force: true }); + } + }); + + it('should abort when report phase is blocked and no onUserInput callback', async () => { + const config = buildConfigWithReport(); + const engine = new PieceEngine(config, tmpDir, 'test task', { projectCwd: tmpDir }); + + // Phase 1 succeeds for plan, then implement + mockRunAgentSequence([ + makeResponse({ persona: 'plan', content: 'Plan done' }), + makeResponse({ persona: 'implement', content: 'Impl done' }), + ]); + + // plan → implement, then implement's report phase blocks + mockDetectMatchedRuleSequence([ + { index: 0, method: 'phase1_tag' }, + ]); + + // Report phase returns blocked (only implement has outputContracts, so only one call) + const blockedResponse = makeResponse({ persona: 'implement', status: 'blocked', content: 'Need clarification for report' }); + vi.mocked(runReportPhase).mockResolvedValueOnce({ blocked: true, response: blockedResponse }); + + const blockedFn = vi.fn(); + const abortFn = vi.fn(); + engine.on('movement:blocked', blockedFn); + engine.on('piece:abort', abortFn); + + const state = await engine.run(); + + expect(state.status).toBe('aborted'); + expect(blockedFn).toHaveBeenCalledOnce(); + expect(abortFn).toHaveBeenCalledOnce(); + }); + + it('should abort when report phase is blocked and onUserInput returns null', async () => { + const config = buildConfigWithReport(); + const onUserInput = vi.fn().mockResolvedValue(null); + const engine = new PieceEngine(config, tmpDir, 'test task', { projectCwd: tmpDir, onUserInput }); + + mockRunAgentSequence([ + makeResponse({ persona: 'plan', content: 'Plan done' }), + makeResponse({ persona: 'implement', content: 'Impl done' }), + ]); + + mockDetectMatchedRuleSequence([ + { index: 0, method: 'phase1_tag' }, + ]); + + const blockedResponse = makeResponse({ persona: 'implement', status: 'blocked', content: 'Need info for report' }); + vi.mocked(runReportPhase).mockResolvedValueOnce({ blocked: true, response: blockedResponse }); + + const state = await engine.run(); + + expect(state.status).toBe('aborted'); + expect(onUserInput).toHaveBeenCalledOnce(); + }); + + it('should retry full movement when report phase is blocked and user provides input', async () => { + const config = buildConfigWithReport(); + const onUserInput = vi.fn().mockResolvedValueOnce('User provided report clarification'); + const engine = new PieceEngine(config, tmpDir, 'test task', { projectCwd: tmpDir, onUserInput }); + + mockRunAgentSequence([ + // First: plan succeeds + makeResponse({ persona: 'plan', content: 'Plan done' }), + // Second: implement Phase 1 succeeds, but Phase 2 will block + makeResponse({ persona: 'implement', content: 'Impl done' }), + // Third: implement retried after user input (Phase 1 re-executes) + makeResponse({ persona: 'implement', content: 'Impl done with clarification' }), + // Fourth: supervise + makeResponse({ persona: 'supervise', content: 'All passed' }), + ]); + + mockDetectMatchedRuleSequence([ + // plan → implement + { index: 0, method: 'phase1_tag' }, + // implement (blocked, no rule eval happens) + // implement retry → supervise + { index: 0, method: 'phase1_tag' }, + // supervise → COMPLETE + { index: 0, method: 'phase1_tag' }, + ]); + + // Report phase: only implement has outputContracts; blocks first, succeeds on retry + const blockedResponse = makeResponse({ persona: 'implement', status: 'blocked', content: 'Need report clarification' }); + vi.mocked(runReportPhase).mockResolvedValueOnce({ blocked: true, response: blockedResponse }); // implement (first attempt) + vi.mocked(runReportPhase).mockResolvedValueOnce(undefined); // implement (retry, succeeds) + + const userInputFn = vi.fn(); + engine.on('movement:user_input', userInputFn); + + const state = await engine.run(); + + expect(state.status).toBe('completed'); + expect(onUserInput).toHaveBeenCalledOnce(); + expect(userInputFn).toHaveBeenCalledOnce(); + expect(state.userInputs).toContain('User provided report clarification'); + }); + + it('should propagate blocked content from report phase to engine response', async () => { + const config = buildConfigWithReport(); + const engine = new PieceEngine(config, tmpDir, 'test task', { projectCwd: tmpDir }); + + mockRunAgentSequence([ + makeResponse({ persona: 'plan', content: 'Plan done' }), + makeResponse({ persona: 'implement', content: 'Original impl content' }), + ]); + + mockDetectMatchedRuleSequence([ + { index: 0, method: 'phase1_tag' }, + ]); + + const blockedContent = 'Blocked: need specific file path for report'; + const blockedResponse = makeResponse({ persona: 'implement', status: 'blocked', content: blockedContent }); + vi.mocked(runReportPhase).mockResolvedValueOnce({ blocked: true, response: blockedResponse }); + + const blockedFn = vi.fn(); + engine.on('movement:blocked', blockedFn); + + const state = await engine.run(); + + expect(state.status).toBe('aborted'); + expect(blockedFn).toHaveBeenCalledWith( + expect.objectContaining({ name: 'implement' }), + expect.objectContaining({ status: 'blocked', content: blockedContent }), + ); + }); +}); diff --git a/src/core/piece/engine/MovementExecutor.ts b/src/core/piece/engine/MovementExecutor.ts index 702560e..9f0d994 100644 --- a/src/core/piece/engine/MovementExecutor.ts +++ b/src/core/piece/engine/MovementExecutor.ts @@ -209,8 +209,15 @@ export class MovementExecutor { const phaseCtx = this.deps.optionsBuilder.buildPhaseRunnerContext(state, response.content, updatePersonaSession, this.deps.onPhaseStart, this.deps.onPhaseComplete); // Phase 2: report output (resume same session, Write only) + // When report phase returns blocked, propagate to PieceEngine's handleBlocked flow if (step.outputContracts && step.outputContracts.length > 0) { - await runReportPhase(step, movementIteration, phaseCtx); + const reportResult = await runReportPhase(step, movementIteration, phaseCtx); + if (reportResult?.blocked) { + response = { ...response, status: 'blocked', content: reportResult.response.content }; + state.movementOutputs.set(step.name, response); + state.lastOutput = response; + return { response, instruction }; + } } // Phase 3: status judgment (resume session, no tools, output status tag) diff --git a/src/core/piece/index.ts b/src/core/piece/index.ts index a2991e2..776c810 100644 --- a/src/core/piece/index.ts +++ b/src/core/piece/index.ts @@ -64,4 +64,4 @@ export { RuleEvaluator, type RuleMatch, type RuleEvaluatorContext, detectMatched export { AggregateEvaluator } from './evaluation/AggregateEvaluator.js'; // Phase runner -export { needsStatusJudgmentPhase, runReportPhase, runStatusJudgmentPhase } from './phase-runner.js'; +export { needsStatusJudgmentPhase, runReportPhase, runStatusJudgmentPhase, type ReportPhaseBlockedResult } from './phase-runner.js'; diff --git a/src/core/piece/phase-runner.ts b/src/core/piece/phase-runner.ts index ac784b5..51b7a93 100644 --- a/src/core/piece/phase-runner.ts +++ b/src/core/piece/phase-runner.ts @@ -7,7 +7,7 @@ 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 { PieceMovement, Language, AgentResponse } from '../models/types.js'; import type { PhaseName } from './types.js'; import { runAgent, type RunAgentOptions } from '../../agents/runner.js'; import { ReportInstructionBuilder } from './instruction/ReportInstructionBuilder.js'; @@ -18,6 +18,9 @@ import { buildSessionKey } from './session-key.js'; const log = createLogger('phase-runner'); +/** Result when Phase 2 encounters a blocked status */ +export type ReportPhaseBlockedResult = { blocked: true; response: AgentResponse }; + export interface PhaseRunnerContext { /** Working directory (agent work dir, may be a clone) */ cwd: string; @@ -107,7 +110,7 @@ export async function runReportPhase( step: PieceMovement, movementIteration: number, ctx: PhaseRunnerContext, -): Promise { +): Promise { const sessionKey = buildSessionKey(step); let currentSessionId = ctx.getSessionId(sessionKey); if (!currentSessionId) { @@ -153,6 +156,11 @@ export async function runReportPhase( throw error; } + if (reportResponse.status === 'blocked') { + ctx.onPhaseComplete?.(step, 2, 'report', reportResponse.content, reportResponse.status); + return { blocked: true, response: reportResponse }; + } + if (reportResponse.status !== 'done') { const errorMsg = reportResponse.error || reportResponse.content || 'Unknown error'; ctx.onPhaseComplete?.(step, 2, 'report', reportResponse.content, reportResponse.status, errorMsg);