From c36a5b1b07eb763db1bc7d596c95ed3751bc0898 Mon Sep 17 00:00:00 2001 From: nrslib <38722970+nrslib@users.noreply.github.com> Date: Sat, 31 Jan 2026 21:53:07 +0900 Subject: [PATCH] resolved #66 --- src/__tests__/engine-abort.test.ts | 205 +++++++++++++++++++++++++++++ src/__tests__/exitCodes.test.ts | 3 + src/commands/workflowExecution.ts | 31 ++++- src/exitCodes.ts | 1 + src/workflow/engine.ts | 29 +++- 5 files changed, 262 insertions(+), 7 deletions(-) create mode 100644 src/__tests__/engine-abort.test.ts diff --git a/src/__tests__/engine-abort.test.ts b/src/__tests__/engine-abort.test.ts new file mode 100644 index 0000000..293882b --- /dev/null +++ b/src/__tests__/engine-abort.test.ts @@ -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(); + }); + }); +}); diff --git a/src/__tests__/exitCodes.test.ts b/src/__tests__/exitCodes.test.ts index e06e219..394e687 100644 --- a/src/__tests__/exitCodes.test.ts +++ b/src/__tests__/exitCodes.test.ts @@ -10,6 +10,7 @@ import { EXIT_WORKFLOW_FAILED, EXIT_GIT_OPERATION_FAILED, EXIT_PR_CREATION_FAILED, + EXIT_SIGINT, } from '../exitCodes.js'; describe('exit codes', () => { @@ -21,6 +22,7 @@ describe('exit codes', () => { EXIT_WORKFLOW_FAILED, EXIT_GIT_OPERATION_FAILED, EXIT_PR_CREATION_FAILED, + EXIT_SIGINT, ]; const unique = new Set(codes); expect(unique.size).toBe(codes.length); @@ -33,5 +35,6 @@ describe('exit codes', () => { expect(EXIT_WORKFLOW_FAILED).toBe(3); expect(EXIT_GIT_OPERATION_FAILED).toBe(4); expect(EXIT_PR_CREATION_FAILED).toBe(5); + expect(EXIT_SIGINT).toBe(130); }); }); diff --git a/src/commands/workflowExecution.ts b/src/commands/workflowExecution.ts index 30e22bd..033d0fa 100644 --- a/src/commands/workflowExecution.ts +++ b/src/commands/workflowExecution.ts @@ -37,6 +37,7 @@ import { import { createLogger } from '../utils/debug.js'; import { notifySuccess, notifyError } from '../utils/notification.js'; import { selectOption, promptInput } from '../prompt/index.js'; +import { EXIT_SIGINT } from '../exitCodes.js'; const log = createLogger('workflow'); @@ -321,10 +322,30 @@ export async function executeWorkflow( notifyError('TAKT', `中断: ${reason}`); }); - const finalState = await engine.run(); - - return { - success: finalState.status === 'completed', - reason: abortReason, + // 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(); + + return { + success: finalState.status === 'completed', + reason: abortReason, + }; + } finally { + process.removeListener('SIGINT', onSigInt); + } } diff --git a/src/exitCodes.ts b/src/exitCodes.ts index b9e7836..723c8b4 100644 --- a/src/exitCodes.ts +++ b/src/exitCodes.ts @@ -11,3 +11,4 @@ export const EXIT_ISSUE_FETCH_FAILED = 2; export const EXIT_WORKFLOW_FAILED = 3; export const EXIT_GIT_OPERATION_FAILED = 4; export const EXIT_PR_CREATION_FAILED = 5; +export const EXIT_SIGINT = 130; // 128 + SIGINT(2), UNIX convention diff --git a/src/workflow/engine.ts b/src/workflow/engine.ts index 1b567e4..4767680 100644 --- a/src/workflow/engine.ts +++ b/src/workflow/engine.ts @@ -29,6 +29,7 @@ import { } from './state-manager.js'; import { generateReportDir } from '../utils/session.js'; import { createLogger } from '../utils/debug.js'; +import { interruptAllQueries } from '../claude/query-manager.js'; const log = createLogger('engine'); @@ -54,6 +55,7 @@ export class WorkflowEngine extends EventEmitter { private loopDetector: LoopDetector; private language: WorkflowEngineOptions['language']; private reportDir: string; + private abortRequested = false; constructor(config: WorkflowConfig, cwd: string, task: string, options: WorkflowEngineOptions = {}) { super(); @@ -145,6 +147,19 @@ export class WorkflowEngine extends EventEmitter { 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 */ private buildInstruction(step: WorkflowStep, stepIteration: number): string { return buildInstructionFromTemplate(step, { @@ -439,6 +454,12 @@ export class WorkflowEngine extends EventEmitter { /** Run the workflow to completion */ async run(): Promise { 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) { this.emit('iteration:limit', this.state.iteration, this.config.maxIterations); @@ -528,9 +549,13 @@ export class WorkflowEngine extends EventEmitter { this.state.currentStep = nextStep; } catch (error) { - const message = error instanceof Error ? error.message : String(error); this.state.status = 'aborted'; - this.emit('workflow:abort', this.state, ERROR_MESSAGES.STEP_EXECUTION_FAILED(message)); + 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)); + } break; } }