resolved #66
This commit is contained in:
parent
3f2971fb72
commit
c36a5b1b07
205
src/__tests__/engine-abort.test.ts
Normal file
205
src/__tests__/engine-abort.test.ts
Normal file
@ -0,0 +1,205 @@
|
|||||||
|
/**
|
||||||
|
* WorkflowEngine tests: abort (SIGINT) scenarios.
|
||||||
|
*
|
||||||
|
* Covers:
|
||||||
|
* - abort() sets state to aborted and emits workflow:abort
|
||||||
|
* - abort() during step execution interrupts the step
|
||||||
|
* - isAbortRequested() reflects abort state
|
||||||
|
* - Double abort() is idempotent
|
||||||
|
*/
|
||||||
|
|
||||||
|
import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
|
||||||
|
import { existsSync, rmSync } from 'node:fs';
|
||||||
|
import type { WorkflowConfig } from '../models/types.js';
|
||||||
|
|
||||||
|
// --- Mock setup (must be before imports that use these modules) ---
|
||||||
|
|
||||||
|
vi.mock('../agents/runner.js', () => ({
|
||||||
|
runAgent: vi.fn(),
|
||||||
|
}));
|
||||||
|
|
||||||
|
vi.mock('../workflow/rule-evaluator.js', () => ({
|
||||||
|
detectMatchedRule: vi.fn(),
|
||||||
|
}));
|
||||||
|
|
||||||
|
vi.mock('../workflow/phase-runner.js', () => ({
|
||||||
|
needsStatusJudgmentPhase: vi.fn().mockReturnValue(false),
|
||||||
|
runReportPhase: vi.fn().mockResolvedValue(undefined),
|
||||||
|
runStatusJudgmentPhase: vi.fn().mockResolvedValue(''),
|
||||||
|
}));
|
||||||
|
|
||||||
|
vi.mock('../utils/session.js', () => ({
|
||||||
|
generateReportDir: vi.fn().mockReturnValue('test-report-dir'),
|
||||||
|
}));
|
||||||
|
|
||||||
|
vi.mock('../claude/query-manager.js', () => ({
|
||||||
|
interruptAllQueries: vi.fn().mockReturnValue(0),
|
||||||
|
}));
|
||||||
|
|
||||||
|
// --- Imports (after mocks) ---
|
||||||
|
|
||||||
|
import { WorkflowEngine } from '../workflow/engine.js';
|
||||||
|
import { runAgent } from '../agents/runner.js';
|
||||||
|
import { interruptAllQueries } from '../claude/query-manager.js';
|
||||||
|
import {
|
||||||
|
makeResponse,
|
||||||
|
makeStep,
|
||||||
|
makeRule,
|
||||||
|
mockRunAgentSequence,
|
||||||
|
mockDetectMatchedRuleSequence,
|
||||||
|
createTestTmpDir,
|
||||||
|
applyDefaultMocks,
|
||||||
|
} from './engine-test-helpers.js';
|
||||||
|
|
||||||
|
describe('WorkflowEngine: Abort (SIGINT)', () => {
|
||||||
|
let tmpDir: string;
|
||||||
|
|
||||||
|
beforeEach(() => {
|
||||||
|
vi.resetAllMocks();
|
||||||
|
applyDefaultMocks();
|
||||||
|
tmpDir = createTestTmpDir();
|
||||||
|
});
|
||||||
|
|
||||||
|
afterEach(() => {
|
||||||
|
if (existsSync(tmpDir)) {
|
||||||
|
rmSync(tmpDir, { recursive: true, force: true });
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
function makeSimpleConfig(): WorkflowConfig {
|
||||||
|
return {
|
||||||
|
name: 'test',
|
||||||
|
maxIterations: 10,
|
||||||
|
initialStep: 'step1',
|
||||||
|
steps: [
|
||||||
|
makeStep('step1', {
|
||||||
|
rules: [
|
||||||
|
makeRule('done', 'step2'),
|
||||||
|
makeRule('fail', 'ABORT'),
|
||||||
|
],
|
||||||
|
}),
|
||||||
|
makeStep('step2', {
|
||||||
|
rules: [
|
||||||
|
makeRule('done', 'COMPLETE'),
|
||||||
|
],
|
||||||
|
}),
|
||||||
|
],
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
describe('abort() before run loop iteration', () => {
|
||||||
|
it('should abort immediately when abort() called before step execution', async () => {
|
||||||
|
const config = makeSimpleConfig();
|
||||||
|
const engine = new WorkflowEngine(config, tmpDir, 'test task');
|
||||||
|
|
||||||
|
const abortFn = vi.fn();
|
||||||
|
engine.on('workflow:abort', abortFn);
|
||||||
|
|
||||||
|
// Call abort before run
|
||||||
|
engine.abort();
|
||||||
|
expect(engine.isAbortRequested()).toBe(true);
|
||||||
|
|
||||||
|
const state = await engine.run();
|
||||||
|
|
||||||
|
expect(state.status).toBe('aborted');
|
||||||
|
expect(abortFn).toHaveBeenCalledOnce();
|
||||||
|
expect(abortFn.mock.calls[0][1]).toContain('SIGINT');
|
||||||
|
// runAgent should never be called since abort was requested before first step
|
||||||
|
expect(vi.mocked(runAgent)).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe('abort() during step execution', () => {
|
||||||
|
it('should abort when abort() is called during runAgent', async () => {
|
||||||
|
const config = makeSimpleConfig();
|
||||||
|
const engine = new WorkflowEngine(config, tmpDir, 'test task');
|
||||||
|
|
||||||
|
// Simulate abort during step execution: runAgent rejects after abort() is called
|
||||||
|
vi.mocked(runAgent).mockImplementation(async () => {
|
||||||
|
engine.abort();
|
||||||
|
throw new Error('Query interrupted');
|
||||||
|
});
|
||||||
|
|
||||||
|
const abortFn = vi.fn();
|
||||||
|
engine.on('workflow:abort', abortFn);
|
||||||
|
|
||||||
|
const state = await engine.run();
|
||||||
|
|
||||||
|
expect(state.status).toBe('aborted');
|
||||||
|
expect(abortFn).toHaveBeenCalledOnce();
|
||||||
|
expect(abortFn.mock.calls[0][1]).toContain('SIGINT');
|
||||||
|
expect(vi.mocked(interruptAllQueries)).toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe('abort() calls interruptAllQueries', () => {
|
||||||
|
it('should call interruptAllQueries when abort() is called', () => {
|
||||||
|
const config = makeSimpleConfig();
|
||||||
|
const engine = new WorkflowEngine(config, tmpDir, 'test task');
|
||||||
|
|
||||||
|
engine.abort();
|
||||||
|
|
||||||
|
expect(vi.mocked(interruptAllQueries)).toHaveBeenCalledOnce();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe('abort() idempotency', () => {
|
||||||
|
it('should only call interruptAllQueries once on multiple abort() calls', () => {
|
||||||
|
const config = makeSimpleConfig();
|
||||||
|
const engine = new WorkflowEngine(config, tmpDir, 'test task');
|
||||||
|
|
||||||
|
engine.abort();
|
||||||
|
engine.abort();
|
||||||
|
engine.abort();
|
||||||
|
|
||||||
|
expect(vi.mocked(interruptAllQueries)).toHaveBeenCalledOnce();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe('isAbortRequested()', () => {
|
||||||
|
it('should return false initially', () => {
|
||||||
|
const config = makeSimpleConfig();
|
||||||
|
const engine = new WorkflowEngine(config, tmpDir, 'test task');
|
||||||
|
|
||||||
|
expect(engine.isAbortRequested()).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should return true after abort()', () => {
|
||||||
|
const config = makeSimpleConfig();
|
||||||
|
const engine = new WorkflowEngine(config, tmpDir, 'test task');
|
||||||
|
|
||||||
|
engine.abort();
|
||||||
|
|
||||||
|
expect(engine.isAbortRequested()).toBe(true);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe('abort between steps', () => {
|
||||||
|
it('should stop after completing current step when abort() is called', async () => {
|
||||||
|
const config = makeSimpleConfig();
|
||||||
|
const engine = new WorkflowEngine(config, tmpDir, 'test task');
|
||||||
|
|
||||||
|
// First step completes normally, but abort is called during it
|
||||||
|
vi.mocked(runAgent).mockImplementation(async () => {
|
||||||
|
// Simulate abort during execution (but the step itself completes)
|
||||||
|
engine.abort();
|
||||||
|
return makeResponse({ agent: 'step1', content: 'Step 1 done' });
|
||||||
|
});
|
||||||
|
|
||||||
|
mockDetectMatchedRuleSequence([
|
||||||
|
{ index: 0, method: 'phase1_tag' }, // step1 → step2
|
||||||
|
]);
|
||||||
|
|
||||||
|
const abortFn = vi.fn();
|
||||||
|
engine.on('workflow:abort', abortFn);
|
||||||
|
|
||||||
|
const state = await engine.run();
|
||||||
|
|
||||||
|
expect(state.status).toBe('aborted');
|
||||||
|
expect(state.iteration).toBe(1);
|
||||||
|
// Only step1 runs; step2 should not start because abort is checked at loop top
|
||||||
|
expect(vi.mocked(runAgent)).toHaveBeenCalledTimes(1);
|
||||||
|
expect(abortFn).toHaveBeenCalledOnce();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
});
|
||||||
@ -10,6 +10,7 @@ import {
|
|||||||
EXIT_WORKFLOW_FAILED,
|
EXIT_WORKFLOW_FAILED,
|
||||||
EXIT_GIT_OPERATION_FAILED,
|
EXIT_GIT_OPERATION_FAILED,
|
||||||
EXIT_PR_CREATION_FAILED,
|
EXIT_PR_CREATION_FAILED,
|
||||||
|
EXIT_SIGINT,
|
||||||
} from '../exitCodes.js';
|
} from '../exitCodes.js';
|
||||||
|
|
||||||
describe('exit codes', () => {
|
describe('exit codes', () => {
|
||||||
@ -21,6 +22,7 @@ describe('exit codes', () => {
|
|||||||
EXIT_WORKFLOW_FAILED,
|
EXIT_WORKFLOW_FAILED,
|
||||||
EXIT_GIT_OPERATION_FAILED,
|
EXIT_GIT_OPERATION_FAILED,
|
||||||
EXIT_PR_CREATION_FAILED,
|
EXIT_PR_CREATION_FAILED,
|
||||||
|
EXIT_SIGINT,
|
||||||
];
|
];
|
||||||
const unique = new Set(codes);
|
const unique = new Set(codes);
|
||||||
expect(unique.size).toBe(codes.length);
|
expect(unique.size).toBe(codes.length);
|
||||||
@ -33,5 +35,6 @@ describe('exit codes', () => {
|
|||||||
expect(EXIT_WORKFLOW_FAILED).toBe(3);
|
expect(EXIT_WORKFLOW_FAILED).toBe(3);
|
||||||
expect(EXIT_GIT_OPERATION_FAILED).toBe(4);
|
expect(EXIT_GIT_OPERATION_FAILED).toBe(4);
|
||||||
expect(EXIT_PR_CREATION_FAILED).toBe(5);
|
expect(EXIT_PR_CREATION_FAILED).toBe(5);
|
||||||
|
expect(EXIT_SIGINT).toBe(130);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@ -37,6 +37,7 @@ import {
|
|||||||
import { createLogger } from '../utils/debug.js';
|
import { createLogger } from '../utils/debug.js';
|
||||||
import { notifySuccess, notifyError } from '../utils/notification.js';
|
import { notifySuccess, notifyError } from '../utils/notification.js';
|
||||||
import { selectOption, promptInput } from '../prompt/index.js';
|
import { selectOption, promptInput } from '../prompt/index.js';
|
||||||
|
import { EXIT_SIGINT } from '../exitCodes.js';
|
||||||
|
|
||||||
const log = createLogger('workflow');
|
const log = createLogger('workflow');
|
||||||
|
|
||||||
@ -321,10 +322,30 @@ export async function executeWorkflow(
|
|||||||
notifyError('TAKT', `中断: ${reason}`);
|
notifyError('TAKT', `中断: ${reason}`);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// SIGINT handler: 1st Ctrl+C = graceful abort, 2nd = force exit
|
||||||
|
let sigintCount = 0;
|
||||||
|
const onSigInt = () => {
|
||||||
|
sigintCount++;
|
||||||
|
if (sigintCount === 1) {
|
||||||
|
console.log();
|
||||||
|
warn('Ctrl+C: ワークフローを中断しています...');
|
||||||
|
engine.abort();
|
||||||
|
} else {
|
||||||
|
console.log();
|
||||||
|
error('Ctrl+C: 強制終了します');
|
||||||
|
process.exit(EXIT_SIGINT);
|
||||||
|
}
|
||||||
|
};
|
||||||
|
process.on('SIGINT', onSigInt);
|
||||||
|
|
||||||
|
try {
|
||||||
const finalState = await engine.run();
|
const finalState = await engine.run();
|
||||||
|
|
||||||
return {
|
return {
|
||||||
success: finalState.status === 'completed',
|
success: finalState.status === 'completed',
|
||||||
reason: abortReason,
|
reason: abortReason,
|
||||||
};
|
};
|
||||||
|
} finally {
|
||||||
|
process.removeListener('SIGINT', onSigInt);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@ -11,3 +11,4 @@ export const EXIT_ISSUE_FETCH_FAILED = 2;
|
|||||||
export const EXIT_WORKFLOW_FAILED = 3;
|
export const EXIT_WORKFLOW_FAILED = 3;
|
||||||
export const EXIT_GIT_OPERATION_FAILED = 4;
|
export const EXIT_GIT_OPERATION_FAILED = 4;
|
||||||
export const EXIT_PR_CREATION_FAILED = 5;
|
export const EXIT_PR_CREATION_FAILED = 5;
|
||||||
|
export const EXIT_SIGINT = 130; // 128 + SIGINT(2), UNIX convention
|
||||||
|
|||||||
@ -29,6 +29,7 @@ import {
|
|||||||
} from './state-manager.js';
|
} from './state-manager.js';
|
||||||
import { generateReportDir } from '../utils/session.js';
|
import { generateReportDir } from '../utils/session.js';
|
||||||
import { createLogger } from '../utils/debug.js';
|
import { createLogger } from '../utils/debug.js';
|
||||||
|
import { interruptAllQueries } from '../claude/query-manager.js';
|
||||||
|
|
||||||
const log = createLogger('engine');
|
const log = createLogger('engine');
|
||||||
|
|
||||||
@ -54,6 +55,7 @@ export class WorkflowEngine extends EventEmitter {
|
|||||||
private loopDetector: LoopDetector;
|
private loopDetector: LoopDetector;
|
||||||
private language: WorkflowEngineOptions['language'];
|
private language: WorkflowEngineOptions['language'];
|
||||||
private reportDir: string;
|
private reportDir: string;
|
||||||
|
private abortRequested = false;
|
||||||
|
|
||||||
constructor(config: WorkflowConfig, cwd: string, task: string, options: WorkflowEngineOptions = {}) {
|
constructor(config: WorkflowConfig, cwd: string, task: string, options: WorkflowEngineOptions = {}) {
|
||||||
super();
|
super();
|
||||||
@ -145,6 +147,19 @@ export class WorkflowEngine extends EventEmitter {
|
|||||||
return this.projectCwd;
|
return this.projectCwd;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/** Request graceful abort: interrupt running queries and stop after current step */
|
||||||
|
abort(): void {
|
||||||
|
if (this.abortRequested) return;
|
||||||
|
this.abortRequested = true;
|
||||||
|
log.info('Abort requested');
|
||||||
|
interruptAllQueries();
|
||||||
|
}
|
||||||
|
|
||||||
|
/** Check if abort has been requested */
|
||||||
|
isAbortRequested(): boolean {
|
||||||
|
return this.abortRequested;
|
||||||
|
}
|
||||||
|
|
||||||
/** Build instruction from template */
|
/** Build instruction from template */
|
||||||
private buildInstruction(step: WorkflowStep, stepIteration: number): string {
|
private buildInstruction(step: WorkflowStep, stepIteration: number): string {
|
||||||
return buildInstructionFromTemplate(step, {
|
return buildInstructionFromTemplate(step, {
|
||||||
@ -439,6 +454,12 @@ export class WorkflowEngine extends EventEmitter {
|
|||||||
/** Run the workflow to completion */
|
/** Run the workflow to completion */
|
||||||
async run(): Promise<WorkflowState> {
|
async run(): Promise<WorkflowState> {
|
||||||
while (this.state.status === 'running') {
|
while (this.state.status === 'running') {
|
||||||
|
if (this.abortRequested) {
|
||||||
|
this.state.status = 'aborted';
|
||||||
|
this.emit('workflow:abort', this.state, 'Workflow interrupted by user (SIGINT)');
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
|
||||||
if (this.state.iteration >= this.config.maxIterations) {
|
if (this.state.iteration >= this.config.maxIterations) {
|
||||||
this.emit('iteration:limit', this.state.iteration, this.config.maxIterations);
|
this.emit('iteration:limit', this.state.iteration, this.config.maxIterations);
|
||||||
|
|
||||||
@ -528,9 +549,13 @@ export class WorkflowEngine extends EventEmitter {
|
|||||||
|
|
||||||
this.state.currentStep = nextStep;
|
this.state.currentStep = nextStep;
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
const message = error instanceof Error ? error.message : String(error);
|
|
||||||
this.state.status = 'aborted';
|
this.state.status = 'aborted';
|
||||||
|
if (this.abortRequested) {
|
||||||
|
this.emit('workflow:abort', this.state, 'Workflow interrupted by user (SIGINT)');
|
||||||
|
} else {
|
||||||
|
const message = error instanceof Error ? error.message : String(error);
|
||||||
this.emit('workflow:abort', this.state, ERROR_MESSAGES.STEP_EXECUTION_FAILED(message));
|
this.emit('workflow:abort', this.state, ERROR_MESSAGES.STEP_EXECUTION_FAILED(message));
|
||||||
|
}
|
||||||
break;
|
break;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user