From d80cb1403e2a03376359dc5a9e197eb47f8d3070 Mon Sep 17 00:00:00 2001 From: Bao Ha Date: Tue, 12 May 2026 22:08:59 +0700 Subject: [PATCH 01/21] feat: [ENG-2776] expose taskId-scoped cancel on agent and session - ChatSession.cancel(taskId?) returns boolean so callers can tell whether a controller was found and aborted vs no-op. - CipherAgent.cancelTask(taskId) fans cancel across live sessions and returns true when any session held the controller. Idempotent. - Legacy no-arg cancel semantics unchanged (REPL Esc path). --- src/agent/core/interfaces/i-chat-session.ts | 10 +- src/agent/core/interfaces/i-cipher-agent.ts | 10 ++ src/agent/infra/agent/cipher-agent.ts | 26 ++++ src/agent/infra/session/chat-session.ts | 21 ++-- .../workspace-scoped-execution.test.ts | 1 + .../agent/cipher-agent-cancel-task.test.ts | 117 ++++++++++++++++++ test/unit/agent/session/chat-session.test.ts | 74 +++++++++++ test/unit/agent/session/index.test.ts | 8 +- 8 files changed, 254 insertions(+), 13 deletions(-) create mode 100644 test/unit/agent/agent/cipher-agent-cancel-task.test.ts diff --git a/src/agent/core/interfaces/i-chat-session.ts b/src/agent/core/interfaces/i-chat-session.ts index 9a5e3ec45..c8aa2b404 100644 --- a/src/agent/core/interfaces/i-chat-session.ts +++ b/src/agent/core/interfaces/i-chat-session.ts @@ -8,10 +8,14 @@ import type {ILLMService} from './i-llm-service.js' */ export interface IChatSession { /** - * Cancel the current operation. - * Aborts any ongoing LLM request. + * Cancel the current operation or a specific task. + * Aborts the abort controller scoped to the given taskId; when no taskId is + * provided, aborts the legacy fallback controller used by interactive runs. + * + * @param taskId - Optional task ID to target a specific in-flight run + * @returns true when a controller was found and aborted, false otherwise */ - cancel(): void + cancel(taskId?: string): boolean /** * Cleanup session resources but preserve history for later restoration. diff --git a/src/agent/core/interfaces/i-cipher-agent.ts b/src/agent/core/interfaces/i-cipher-agent.ts index 39528a38b..1ab6ccbe4 100644 --- a/src/agent/core/interfaces/i-cipher-agent.ts +++ b/src/agent/core/interfaces/i-cipher-agent.ts @@ -83,6 +83,16 @@ export interface ICipherAgent { */ cancel(): Promise + /** + * Cancels a specific in-flight task by fanning the cancel across all live + * sessions. Idempotent: a second call for the same taskId returns false + * because the controller has already been aborted and removed. + * + * @param taskId - Task identifier (matches the taskId passed to run/streamRun) + * @returns true when any session held a controller for the task; false otherwise + */ + cancelTask(taskId: string): Promise + /** * Create a task-scoped child session for parallel execution. * The session gets its own sandbox, context manager, and LLM service. diff --git a/src/agent/infra/agent/cipher-agent.ts b/src/agent/infra/agent/cipher-agent.ts index 588bebc92..bc3b719c2 100644 --- a/src/agent/infra/agent/cipher-agent.ts +++ b/src/agent/infra/agent/cipher-agent.ts @@ -209,6 +209,32 @@ export class CipherAgent extends BaseAgent implements ICipherAgent { return Boolean(streamController) } + /** + * Cancel a specific in-flight task across all live sessions. + * Fans the cancel out to every session managed by this agent; returns true + * as soon as at least one session reports it held the controller. + * Idempotent — a second call for the same taskId returns false because the + * controller has already been aborted and removed from its session. + * + * @param taskId - Task identifier (matches the taskId passed to run/streamRun) + * @returns true when any session cancelled the task, false when no session held it + */ + public async cancelTask(taskId: string): Promise { + this.ensureStarted() + + const sessionManager = this.getSessionManagerInternal() + let cancelled = false + for (const sessionId of sessionManager.listSessions()) { + const session = sessionManager.getSession(sessionId) + if (!session) continue + if (session.cancel(taskId)) { + cancelled = true + } + } + + return cancelled + } + // === Public Methods (alphabetical order) === protected override async cleanupServices(): Promise { diff --git a/src/agent/infra/session/chat-session.ts b/src/agent/infra/session/chat-session.ts index 8e809e39b..b2567f778 100644 --- a/src/agent/infra/session/chat-session.ts +++ b/src/agent/infra/session/chat-session.ts @@ -95,18 +95,23 @@ export class ChatSession implements IChatSession { /** * Cancel the current operation or a specific task. - * @param taskId - Optional taskId to cancel specific task, otherwise cancels fallback controller + * + * @param taskId - Optional taskId to cancel a specific task, otherwise cancels the fallback controller + * @returns true when a controller was found and aborted, false otherwise (so callers can decide + * whether to emit a terminal event upstream) */ - public cancel(taskId?: string): void { + public cancel(taskId?: string): boolean { if (taskId) { const controller = this.activeControllers.get(taskId) - if (controller) { - controller.abort() - this.activeControllers.delete(taskId) - } - } else if (this.currentController) { - this.currentController.abort() + if (!controller) return false + controller.abort() + this.activeControllers.delete(taskId) + return true } + + if (!this.currentController) return false + this.currentController.abort() + return true } /** diff --git a/test/integration/workspace/workspace-scoped-execution.test.ts b/test/integration/workspace/workspace-scoped-execution.test.ts index 35978d518..3469d6531 100644 --- a/test/integration/workspace/workspace-scoped-execution.test.ts +++ b/test/integration/workspace/workspace-scoped-execution.test.ts @@ -46,6 +46,7 @@ function makeStubAgent(sandbox: SinonSandbox): ICipherAgent & { } { return { cancel: sandbox.stub().resolves(false), + cancelTask: sandbox.stub().resolves(false), createTaskSession: sandbox.stub().resolves('task-session-1'), deleteSandboxVariable: sandbox.stub(), deleteSandboxVariableOnSession: sandbox.stub(), diff --git a/test/unit/agent/agent/cipher-agent-cancel-task.test.ts b/test/unit/agent/agent/cipher-agent-cancel-task.test.ts new file mode 100644 index 000000000..c2fa23178 --- /dev/null +++ b/test/unit/agent/agent/cipher-agent-cancel-task.test.ts @@ -0,0 +1,117 @@ +import {expect} from 'chai' +import {restore, stub} from 'sinon' + +import type {AgentConfig} from '../../../../src/agent/infra/agent/index.js' + +import {CipherAgent} from '../../../../src/agent/infra/agent/index.js' + +describe('CipherAgent.cancelTask', () => { + let agentConfig: AgentConfig + + beforeEach(() => { + agentConfig = { + apiBaseUrl: 'http://localhost:3333', + blobStorage: { + maxBlobSize: 100 * 1024 * 1024, + maxTotalSize: 1024 * 1024 * 1024, + storageDir: '/tmp/brv-test-blobs', + }, + llm: { + maxIterations: 10, + maxTokens: 1000, + temperature: 0.5, + }, + model: 'gemini-2.5-flash', + projectId: 'byterover', + sessionKey: 'test-session-key', + storagePath: '/tmp/brv-test-storage', + } + stub(console, 'log') + }) + + afterEach(() => { + restore() + }) + + it('returns false when no session holds the taskId', async () => { + const agent = new CipherAgent(agentConfig) + await agent.start() + try { + const result = await agent.cancelTask('unknown-task') + expect(result).to.equal(false) + } finally { + await agent.stop() + } + }) + + it('returns true when at least one session cancels the taskId', async () => { + const agent = new CipherAgent(agentConfig) + await agent.start() + try { + const sessions = agent.listSessions() + expect(sessions.length).to.be.greaterThan(0) + const session = agent.getSession(sessions[0]) + expect(session).to.not.be.undefined + stub(session!, 'cancel').returns(true) + + const result = await agent.cancelTask('task-A') + expect(result).to.equal(true) + } finally { + await agent.stop() + } + }) + + it('returns true when any one of several sessions cancels the taskId', async () => { + const agent = new CipherAgent(agentConfig) + await agent.start() + try { + await agent.createSession('extra-1') + await agent.createSession('extra-2') + + const sessions = agent.listSessions() + expect(sessions.length).to.be.gte(3) + + // First two sessions report false, last reports true. + const last = sessions.at(-1)! + for (const id of sessions) { + const s = agent.getSession(id)! + stub(s, 'cancel').returns(id === last) + } + + const result = await agent.cancelTask('task-B') + expect(result).to.equal(true) + } finally { + await agent.stop() + } + }) + + it('is idempotent: second call returns false after the controller is gone', async () => { + const agent = new CipherAgent(agentConfig) + await agent.start() + try { + const sessions = agent.listSessions() + const session = agent.getSession(sessions[0])! + const cancelStub = stub(session, 'cancel') + cancelStub.onFirstCall().returns(true) + cancelStub.onSecondCall().returns(false) + + const first = await agent.cancelTask('task-C') + const second = await agent.cancelTask('task-C') + + expect(first).to.equal(true) + expect(second).to.equal(false) + } finally { + await agent.stop() + } + }) + + it('throws if called before start()', async () => { + const agent = new CipherAgent(agentConfig) + try { + await agent.cancelTask('task-X') + expect.fail('Should have thrown') + } catch (error) { + expect((error as Error).message).to.include('must be started') + } + }) +}) diff --git a/test/unit/agent/session/chat-session.test.ts b/test/unit/agent/session/chat-session.test.ts index cbf3a31a8..15ff48b30 100644 --- a/test/unit/agent/session/chat-session.test.ts +++ b/test/unit/agent/session/chat-session.test.ts @@ -445,6 +445,80 @@ describe('ChatSession', () => { it('should not throw when no currentController exists', () => { expect(() => session.cancel()).to.not.throw() }) + + it('should return true when fallback controller is aborted', async () => { + ;(mockLLMService.completeTask as SinonStub).callsFake(async (_input, options) => { + await new Promise((resolve) => { + const signal = options?.signal as AbortSignal + signal.addEventListener('abort', () => resolve(), {once: true}) + }) + return 'response' + }) + + const runPromise = session.run('input') + const result = session.cancel() + await runPromise.catch(() => {}) + + expect(result).to.equal(true) + }) + + it('should return false when no controller exists for the given taskId', () => { + expect(session.cancel('unknown-task')).to.equal(false) + }) + + it('should return false when no fallback controller and no taskId given', () => { + expect(session.cancel()).to.equal(false) + }) + + it('should abort only the controller matching the given taskId', async () => { + const aborts: Record = {a: false, b: false} + ;(mockLLMService.completeTask as SinonStub).callsFake(async (_input, options) => { + const taskId = options?.taskId as string + const signal = options?.signal as AbortSignal + await new Promise((resolve) => { + signal.addEventListener('abort', () => { + aborts[taskId] = true + resolve() + }, {once: true}) + }) + return 'response' + }) + + const runA = session.run('a', {taskId: 'a'}) + const runB = session.run('b', {taskId: 'b'}) + + const result = session.cancel('a') + await runA.catch(() => {}) + + // After cancelling A, B's controller must still be present and B must still + // be pending. Cancelling B explicitly lets the test finish. + expect(aborts.a).to.equal(true) + expect(aborts.b).to.equal(false) + + session.cancel('b') + await runB.catch(() => {}) + + expect(result).to.equal(true) + expect(aborts.b).to.equal(true) + }) + + it('should be idempotent: second cancel for the same taskId returns false', async () => { + ;(mockLLMService.completeTask as SinonStub).callsFake(async (_input, options) => { + const signal = options?.signal as AbortSignal + await new Promise((resolve) => { + signal.addEventListener('abort', () => resolve(), {once: true}) + }) + return 'response' + }) + + const runPromise = session.run('input', {taskId: 'task-1'}) + const first = session.cancel('task-1') + const second = session.cancel('task-1') + await runPromise.catch(() => {}) + + expect(first).to.equal(true) + expect(second).to.equal(false) + }) }) describe('dispose()', () => { diff --git a/test/unit/agent/session/index.test.ts b/test/unit/agent/session/index.test.ts index 628d3e9b5..f1b8736e6 100644 --- a/test/unit/agent/session/index.test.ts +++ b/test/unit/agent/session/index.test.ts @@ -92,7 +92,9 @@ describe('Session index exports', () => { it('should export IChatSession interface', () => { // Type check - if this compiles, the interface is exported const session: IChatSession = { - cancel() {}, + cancel() { + return false + }, cleanup() {}, dispose() {}, getHistory() { @@ -185,7 +187,9 @@ describe('Session index exports', () => { // Interfaces are checked above via type annotations // Cannot test interfaces as values, but compilation ensures they exist const session: IChatSession = { - cancel() {}, + cancel() { + return false + }, cleanup() {}, dispose() {}, getHistory() { From 9647f878aea5ed3d8d1bb140c4e7a016565a2f29 Mon Sep 17 00:00:00 2001 From: Bao Ha Date: Tue, 12 May 2026 23:10:02 +0700 Subject: [PATCH 02/21] feat: [ENG-2775] agent process subscribes to task:cancel - New handleAgentCancelEvent helper delegates to CipherAgent.cancelTask and emits task:cancelled upstream only when an active controller was held. Best-effort: any error from agent or transport is logged and swallowed so the cancel pipeline never crashes the agent. - agent-process registers the listener alongside task:execute. - Unit tests cover matched emit, unmatched no-op, agent-throw and transport-throw paths, idempotency, and per-event logging. --- .../infra/daemon/agent-cancel-listener.ts | 47 ++++++++ src/server/infra/daemon/agent-process.ts | 8 ++ .../daemon/agent-cancel-listener.test.ts | 106 ++++++++++++++++++ 3 files changed, 161 insertions(+) create mode 100644 src/server/infra/daemon/agent-cancel-listener.ts create mode 100644 test/unit/infra/daemon/agent-cancel-listener.test.ts diff --git a/src/server/infra/daemon/agent-cancel-listener.ts b/src/server/infra/daemon/agent-cancel-listener.ts new file mode 100644 index 000000000..c4ff19147 --- /dev/null +++ b/src/server/infra/daemon/agent-cancel-listener.ts @@ -0,0 +1,47 @@ +/** + * Agent cancel listener helper. + * + * Pure handler logic extracted from agent-process.ts for testability. + * Given a cancel request, asks the agent to cancel and conditionally + * emits `task:cancelled` upstream. Best-effort by design: any failure is + * logged and swallowed so the cancel pipeline never crashes the agent. + */ + +import type {ITransportClient} from '@campfirein/brv-transport-client' + +import type {ICipherAgent} from '../../../agent/core/interfaces/i-cipher-agent.js' + +import {TransportTaskEventNames} from '../../core/domain/transport/schemas.js' + +export type HandleAgentCancelEventOptions = { + agent: Pick + log: (msg: string) => void + taskId: string + transport: Pick +} + +/** + * Handle one `task:cancel` event inside the agent child process. + * + * Behavior: + * - Calls `agent.cancelTask(taskId)`. + * - On a truthy result, emits `task:cancelled` back to the daemon so it can + * broadcast to clients and run lifecycle hooks. + * - On a falsy result (no controller held the task), stays silent: the daemon + * reconciles via its own state. + * - Any error from the agent or the transport is logged and suppressed. + * + * @param options - taskId + agent/transport/log collaborators + */ +export async function handleAgentCancelEvent(options: HandleAgentCancelEventOptions): Promise { + const {agent, log, taskId, transport} = options + log(`task:cancel received taskId=${taskId}`) + try { + const cancelled = await agent.cancelTask(taskId) + if (!cancelled) return + transport.request(TransportTaskEventNames.CANCELLED, {taskId}) + } catch (error) { + const message = error instanceof Error ? error.message : String(error) + log(`task:cancel handler error taskId=${taskId} err=${message}`) + } +} diff --git a/src/server/infra/daemon/agent-process.ts b/src/server/infra/daemon/agent-process.ts index 41cd7e9e6..6a28423ef 100644 --- a/src/server/infra/daemon/agent-process.ts +++ b/src/server/infra/daemon/agent-process.ts @@ -25,6 +25,7 @@ import {appendFileSync} from 'node:fs' import {join} from 'node:path' import type {ISearchKnowledgeService} from '../../../agent/infra/sandbox/tools-sdk.js' +import type {TaskCancelRequest} from '../../../shared/transport/events/task-events.js' import type {BrvConfig} from '../../core/domain/entities/brv-config.js' import type {ProviderConfigResponse, TaskExecute} from '../../core/domain/transport/schemas.js' import type {IRuntimeSignalStore} from '../../core/interfaces/storage/i-runtime-signal-store.js' @@ -63,6 +64,7 @@ import {SearchExecutor} from '../executor/search-executor.js' import {FileCurateLogStore} from '../storage/file-curate-log-store.js' import {FileReviewBackupStore} from '../storage/file-review-backup-store.js' import {AgentInstanceDiscovery} from '../transport/agent-instance-discovery.js' +import {handleAgentCancelEvent} from './agent-cancel-listener.js' import {createAgentLogger} from './agent-logger.js' import {PostWorkRegistry} from './post-work-registry.js' import {resolveSessionId} from './session-resolver.js' @@ -429,6 +431,12 @@ async function start(): Promise { ) }) + transport.on(TransportTaskEventNames.CANCEL, ({taskId}) => { + if (!agent || !transport) return + // eslint-disable-next-line no-void + void handleAgentCancelEvent({agent, log: agentLog, taskId, transport}) + }) + // 8. Register with transport server (for TransportHandlers tracking) await transport.requestWithAck('agent:register', {projectPath}) diff --git a/test/unit/infra/daemon/agent-cancel-listener.test.ts b/test/unit/infra/daemon/agent-cancel-listener.test.ts new file mode 100644 index 000000000..bb0e784e4 --- /dev/null +++ b/test/unit/infra/daemon/agent-cancel-listener.test.ts @@ -0,0 +1,106 @@ +import type {ITransportClient} from '@campfirein/brv-transport-client' + +import {expect} from 'chai' +import {createSandbox, SinonSandbox, SinonStub} from 'sinon' + +import type {ICipherAgent} from '../../../../src/agent/core/interfaces/i-cipher-agent.js' + +import {handleAgentCancelEvent} from '../../../../src/server/infra/daemon/agent-cancel-listener.js' + +type CancelDeps = { + agent: Pick + log: (msg: string) => void + taskId: string + transport: Pick +} + +describe('handleAgentCancelEvent', () => { + let sandbox: SinonSandbox + let cancelTaskStub: SinonStub + let requestStub: SinonStub + let logCalls: string[] + let deps: Omit + + beforeEach(() => { + sandbox = createSandbox() + cancelTaskStub = sandbox.stub() + requestStub = sandbox.stub() + logCalls = [] + deps = { + agent: {cancelTask: cancelTaskStub}, + log: (msg) => logCalls.push(msg), + transport: {request: requestStub}, + } + }) + + afterEach(() => { + sandbox.restore() + }) + + it('emits task:cancelled when the agent reports it cancelled the task', async () => { + cancelTaskStub.resolves(true) + + await handleAgentCancelEvent({taskId: 'task-1', ...deps}) + + expect(cancelTaskStub.calledOnceWithExactly('task-1')).to.equal(true) + expect(requestStub.calledOnceWithExactly('task:cancelled', {taskId: 'task-1'})).to.equal(true) + }) + + it('does NOT emit task:cancelled when the agent reports no controller was held', async () => { + cancelTaskStub.resolves(false) + + await handleAgentCancelEvent({taskId: 'unknown-task', ...deps}) + + expect(cancelTaskStub.calledOnceWithExactly('unknown-task')).to.equal(true) + expect(requestStub.called).to.equal(false) + }) + + it('does NOT emit, never throws when agent.cancelTask rejects (best-effort)', async () => { + cancelTaskStub.rejects(new Error('agent boom')) + + let thrown: unknown + try { + await handleAgentCancelEvent({taskId: 'task-err', ...deps}) + } catch (error) { + thrown = error + } + + expect(thrown).to.equal(undefined) + expect(requestStub.called).to.equal(false) + expect(logCalls.some((l) => l.includes('task-err') && l.toLowerCase().includes('err'))).to.equal(true) + }) + + it('does NOT emit, never throws when transport.request throws synchronously', async () => { + cancelTaskStub.resolves(true) + requestStub.throws(new Error('transport boom')) + + let thrown: unknown + try { + await handleAgentCancelEvent({taskId: 'task-2', ...deps}) + } catch (error) { + thrown = error + } + + expect(thrown).to.equal(undefined) + expect(logCalls.some((l) => l.includes('task-2') && l.toLowerCase().includes('err'))).to.equal(true) + }) + + it('logs receipt of every cancel request (matched or not)', async () => { + cancelTaskStub.resolves(false) + + await handleAgentCancelEvent({taskId: 'log-test', ...deps}) + + expect(logCalls.some((l) => l.includes('log-test'))).to.equal(true) + }) + + it('is idempotent: second call after the controller is gone returns silently', async () => { + cancelTaskStub.onFirstCall().resolves(true) + cancelTaskStub.onSecondCall().resolves(false) + + await handleAgentCancelEvent({taskId: 'task-idem', ...deps}) + await handleAgentCancelEvent({taskId: 'task-idem', ...deps}) + + expect(requestStub.callCount).to.equal(1) + expect(requestStub.firstCall.args[1]).to.deep.equal({taskId: 'task-idem'}) + }) +}) From 09f59e2d0361e87209471826fea58f4dc5a019f9 Mon Sep 17 00:00:00 2001 From: Bao Ha Date: Wed, 13 May 2026 00:18:51 +0700 Subject: [PATCH 03/21] feat: [ENG-2777] cancel-aware execute, queue advance, queued-task cancel - handleExecutorTerminalError helper branches the executor catch: SessionCancelledError suppresses task:error (T1.1 emits task:cancelled); anything else takes the existing task:error path. Curate Phase 4 is naturally skipped because the executor throws before postWork is assigned; dream's finally still releases the lock via rollback on the abort path. - task-router.handleTaskCancelled now calls agentPool.notifyTaskCompleted so the project FIFO drains after a cancel, symmetric with the completed/error paths. - New IAgentPool.cancelQueuedTask delegates to ProjectTaskQueue.cancel. task-router.handleTaskCancel checks the queue first; queued tasks are removed and the daemon emits task:cancelled directly, never forwarding to the agent that holds no controller for them. - Extract cancelTaskLocally to share broadcast and hook plumbing between the queued-cancel and no-agent-connected branches. - Tests: 5 for handleExecutorTerminalError, 4 for task-router cancel paths, 3 for agent-pool cancelQueuedTask. Update 7 existing mock IAgentPool to satisfy the new interface method. --- .../core/interfaces/agent/i-agent-pool.ts | 12 ++ .../infra/daemon/agent-executor-error.ts | 53 +++++++++ src/server/infra/daemon/agent-pool.ts | 9 ++ src/server/infra/daemon/agent-process.ts | 12 +- src/server/infra/process/task-router.ts | 45 +++++-- .../process/task-router-extended.test.ts | 1 + .../scenarios/task-history-webui-flow.test.ts | 1 + test/integration/taskid-flow.test.ts | 3 + .../workspace/resolver-to-task-router.test.ts | 1 + .../infra/daemon/agent-executor-error.test.ts | 110 ++++++++++++++++++ test/unit/infra/daemon/agent-pool.test.ts | 36 ++++++ .../process/connection-coordinator.test.ts | 1 + .../process/task-router-accumulator.test.ts | 1 + test/unit/infra/process/task-router.test.ts | 61 ++++++++++ .../infra/process/transport-handlers.test.ts | 1 + 15 files changed, 326 insertions(+), 21 deletions(-) create mode 100644 src/server/infra/daemon/agent-executor-error.ts create mode 100644 test/unit/infra/daemon/agent-executor-error.test.ts diff --git a/src/server/core/interfaces/agent/i-agent-pool.ts b/src/server/core/interfaces/agent/i-agent-pool.ts index 4c8d97560..823453314 100644 --- a/src/server/core/interfaces/agent/i-agent-pool.ts +++ b/src/server/core/interfaces/agent/i-agent-pool.ts @@ -40,6 +40,18 @@ export type AgentEntryInfo = { * - ClientManager.onProjectEmpty → markIdle() for idle cleanup */ export interface IAgentPool { + /** + * Remove a queued task by taskId from any project's FIFO queue. + * Returns true when the task was found and removed; false when the task + * is not in any queue (either it never queued, or it is currently in flight). + * + * Used by the cancel pipeline to short-circuit queued cancellations without + * forwarding the request to an agent that has no controller for the task. + * + * @param taskId - Task identifier to remove from queues + */ + cancelQueuedTask(taskId: string): boolean + /** * Get pool entries for monitoring/debugging. */ diff --git a/src/server/infra/daemon/agent-executor-error.ts b/src/server/infra/daemon/agent-executor-error.ts new file mode 100644 index 000000000..99c8c69e3 --- /dev/null +++ b/src/server/infra/daemon/agent-executor-error.ts @@ -0,0 +1,53 @@ +/** + * Agent executor terminal-error handler. + * + * Decides between two terminal events when the task body throws: + * - `SessionCancelledError` → suppress `task:error` (T1.1's cancel listener + * has already emitted `task:cancelled`). Returns silently so the persisted + * history reflects the daemon's `status: 'cancelled'` instead of `error`. + * - Anything else → emit `task:error` with the usual serialized + * payload. + * + * Extracted from agent-process.ts so the dispatch logic is unit-testable. + */ + +import type {ITransportClient} from '@campfirein/brv-transport-client' + +import {SessionCancelledError} from '../../../agent/core/domain/errors/session-error.js' +import {serializeTaskError} from '../../core/domain/errors/task-error.js' +import {TransportTaskEventNames} from '../../core/domain/transport/schemas.js' + +export type HandleExecutorTerminalErrorOptions = { + clientId: string + error: unknown + log: (msg: string) => void + projectPath: string + taskId: string + transport: Pick +} + +/** + * Map one executor throw to the right terminal transport event. + * Cancellation is treated as a deliberate end, not a failure: no `task:error` + * is emitted on that path. The cancel listener in T1.1 owns the + * `task:cancelled` emission, so this handler stays silent on cancel. + * + * @param options - error + identifiers + transport/log collaborators + */ +export function handleExecutorTerminalError(options: HandleExecutorTerminalErrorOptions): void { + const {clientId, error, log, projectPath, taskId, transport} = options + + if (error instanceof SessionCancelledError) { + log(`task cancelled mid-execute taskId=${taskId} — suppressing task:error (cancel listener emits task:cancelled)`) + return + } + + const errorData = serializeTaskError(error) + log(`task:error taskId=${taskId} error=${errorData.message}`) + try { + transport.request(TransportTaskEventNames.ERROR, {clientId, error: errorData, projectPath, taskId}) + } catch (sendError) { + const message = sendError instanceof Error ? sendError.message : String(sendError) + log(`task:error send failed taskId=${taskId}: ${message}`) + } +} diff --git a/src/server/infra/daemon/agent-pool.ts b/src/server/infra/daemon/agent-pool.ts index 3e9e32fe9..35804d02d 100644 --- a/src/server/infra/daemon/agent-pool.ts +++ b/src/server/infra/daemon/agent-pool.ts @@ -108,6 +108,15 @@ export class AgentPool implements IAgentPool { this.transportServer = options.transportServer } + /** + * Remove a queued task by id from any project's FIFO. Returns true when the + * task was found and removed; false when the task is not in any queue. + * Tasks that are mid-execution on an agent are not visible here. + */ + cancelQueuedTask(taskId: string): boolean { + return this.taskQueue.cancel(taskId) + } + getEntries(): readonly AgentEntryInfo[] { return [...this.agents.values()].map((entry) => ({ activeTasks: entry.agent.activeTasks, diff --git a/src/server/infra/daemon/agent-process.ts b/src/server/infra/daemon/agent-process.ts index 6a28423ef..b08d5f0cc 100644 --- a/src/server/infra/daemon/agent-process.ts +++ b/src/server/infra/daemon/agent-process.ts @@ -65,6 +65,7 @@ import {FileCurateLogStore} from '../storage/file-curate-log-store.js' import {FileReviewBackupStore} from '../storage/file-review-backup-store.js' import {AgentInstanceDiscovery} from '../transport/agent-instance-discovery.js' import {handleAgentCancelEvent} from './agent-cancel-listener.js' +import {handleExecutorTerminalError} from './agent-executor-error.js' import {createAgentLogger} from './agent-logger.js' import {PostWorkRegistry} from './post-work-registry.js' import {resolveSessionId} from './session-resolver.js' @@ -681,16 +682,7 @@ async function executeTask( postWorkRegistry.submit(projectPath, postWork) } } catch (error) { - // Emit task:error - const errorData = serializeTaskError(error) - agentLog(`task:error taskId=${taskId} error=${errorData.message}`) - try { - transport.request(TransportTaskEventNames.ERROR, {clientId, error: errorData, projectPath, taskId}) - } catch (error_) { - agentLog( - `task:error send failed taskId=${taskId}: ${error_ instanceof Error ? error_.message : String(error_)}`, - ) - } + handleExecutorTerminalError({clientId, error, log: agentLog, projectPath, taskId, transport}) } finally { cleanupForwarding?.() } diff --git a/src/server/infra/process/task-router.ts b/src/server/infra/process/task-router.ts index 881734437..94f4a13d9 100644 --- a/src/server/infra/process/task-router.ts +++ b/src/server/infra/process/task-router.ts @@ -590,6 +590,20 @@ export class TaskRouter { }) } + private cancelTaskLocally(task: TaskInfo, taskId: string): void { + this.transport.sendTo(task.clientId, TransportTaskEventNames.CANCELLED, {taskId}) + broadcastToProjectRoom( + this.projectRegistry, + this.projectRouter, + task.projectPath, + TransportTaskEventNames.CANCELLED, + {taskId}, + task.clientId, + ) + this.tasks.delete(taskId) + this.notifyHooksCancelled(taskId, task).catch(() => {}) + } + /** * Drain the dirty set: for each taskId still active, fire `onTaskUpdate` on * each lifecycle hook. Tasks moved to `completedTasks` between markDirty @@ -628,6 +642,17 @@ export class TaskRouter { return {error: 'Task not found', success: false} } + // Queue-first: a task that has not been dispatched to an agent still lives + // in the pool's per-project FIFO. Removing it there avoids forwarding a + // cancel to an agent that holds no controller for the task. The daemon + // emits the terminal event itself and runs lifecycle hooks so history + // reflects status: 'cancelled' on this path too. + if (this.agentPool?.cancelQueuedTask(taskId)) { + transportLog(`Cancelled queued task: ${taskId}`) + this.cancelTaskLocally(task, taskId) + return {success: true} + } + // If Agent connected for this task's project, forward cancel request const agentId = this.getAgentForProject(task.projectPath) if (agentId) { @@ -637,17 +662,7 @@ export class TaskRouter { // No Agent - cancel task locally and emit terminal event transportLog(`No Agent connected, cancelling task locally: ${taskId}`) - this.transport.sendTo(task.clientId, TransportTaskEventNames.CANCELLED, {taskId}) - broadcastToProjectRoom( - this.projectRegistry, - this.projectRouter, - task.projectPath, - TransportTaskEventNames.CANCELLED, - {taskId}, - task.clientId, - ) - this.tasks.delete(taskId) - this.notifyHooksCancelled(taskId, task).catch(() => {}) + this.cancelTaskLocally(task, taskId) return {success: true} } @@ -677,6 +692,14 @@ export class TaskRouter { ) this.moveToCompleted(taskId) + // Notify pool so the project queue drains. Symmetric with task:completed + // and task:error — cancellation also vacates a pool slot, and without this + // call the next queued task for the project would wait until another task + // finishes naturally. + if (task?.projectPath) { + this.agentPool?.notifyTaskCompleted(task.projectPath) + } + // Notify hooks (fire-and-forget) if (task) { this.notifyHooksCancelled(taskId, task).catch(() => {}) diff --git a/test/integration/infra/process/task-router-extended.test.ts b/test/integration/infra/process/task-router-extended.test.ts index 6a67dc797..f5a543128 100644 --- a/test/integration/infra/process/task-router-extended.test.ts +++ b/test/integration/infra/process/task-router-extended.test.ts @@ -67,6 +67,7 @@ function makeStubTransportServer(sandbox: SinonSandbox) { function makeStubAgentPool(sandbox: SinonSandbox): IAgentPool { return { + cancelQueuedTask: sandbox.stub().returns(false), getEntries: sandbox.stub().returns([]), getSize: sandbox.stub().returns(0), handleAgentDisconnected: sandbox.stub(), diff --git a/test/integration/scenarios/task-history-webui-flow.test.ts b/test/integration/scenarios/task-history-webui-flow.test.ts index 2ed77d554..cc5ccd6ae 100644 --- a/test/integration/scenarios/task-history-webui-flow.test.ts +++ b/test/integration/scenarios/task-history-webui-flow.test.ts @@ -76,6 +76,7 @@ function makeStubTransport(sandbox: SinonSandbox) { function makeStubAgentPool(sandbox: SinonSandbox): IAgentPool { return { + cancelQueuedTask: sandbox.stub().returns(false), getEntries: sandbox.stub().returns([]), getSize: sandbox.stub().returns(0), handleAgentDisconnected: sandbox.stub(), diff --git a/test/integration/taskid-flow.test.ts b/test/integration/taskid-flow.test.ts index 01a0e6152..19c027891 100644 --- a/test/integration/taskid-flow.test.ts +++ b/test/integration/taskid-flow.test.ts @@ -52,6 +52,9 @@ describe('TaskId Integration Flow', () => { // Stub agentPool that forwards tasks to the mock agent via the real transport server const stubAgentPool: IAgentPool = { + cancelQueuedTask() { + return false + }, getEntries() { return [] }, diff --git a/test/integration/workspace/resolver-to-task-router.test.ts b/test/integration/workspace/resolver-to-task-router.test.ts index 53dc42c15..3c241476d 100644 --- a/test/integration/workspace/resolver-to-task-router.test.ts +++ b/test/integration/workspace/resolver-to-task-router.test.ts @@ -55,6 +55,7 @@ function makeStubTransportServer(sandbox: SinonSandbox) { function makeStubAgentPool(sandbox: SinonSandbox): IAgentPool & {submitTask: SinonStub} { return { + cancelQueuedTask: sandbox.stub().returns(false), getEntries: sandbox.stub().returns([]), getSize: sandbox.stub().returns(0), handleAgentDisconnected: sandbox.stub(), diff --git a/test/unit/infra/daemon/agent-executor-error.test.ts b/test/unit/infra/daemon/agent-executor-error.test.ts new file mode 100644 index 000000000..9339cc26a --- /dev/null +++ b/test/unit/infra/daemon/agent-executor-error.test.ts @@ -0,0 +1,110 @@ +import type {ITransportClient} from '@campfirein/brv-transport-client' + +import {expect} from 'chai' +import {createSandbox, SinonSandbox, SinonStub} from 'sinon' + +import {SessionCancelledError} from '../../../../src/agent/core/domain/errors/session-error.js' +import {handleExecutorTerminalError} from '../../../../src/server/infra/daemon/agent-executor-error.js' + +describe('handleExecutorTerminalError', () => { + let sandbox: SinonSandbox + let requestStub: SinonStub + let logCalls: string[] + let transport: Pick + + beforeEach(() => { + sandbox = createSandbox() + requestStub = sandbox.stub() + logCalls = [] + transport = {request: requestStub} + }) + + afterEach(() => { + sandbox.restore() + }) + + it('does NOT emit task:error when error is SessionCancelledError (T1.1 already emitted task:cancelled)', () => { + const error = new SessionCancelledError('session-1') + + handleExecutorTerminalError({ + clientId: 'client-1', + error, + log: (msg: string) => logCalls.push(msg), + projectPath: '/proj', + taskId: 'task-cancel', + transport, + }) + + expect(requestStub.called).to.equal(false) + expect(logCalls.some((l) => l.includes('task-cancel') && l.toLowerCase().includes('cancel'))).to.equal(true) + }) + + it('emits task:error for non-cancellation errors', () => { + const error = new Error('boom') + + handleExecutorTerminalError({ + clientId: 'client-1', + error, + log: (msg: string) => logCalls.push(msg), + projectPath: '/proj', + taskId: 'task-err', + transport, + }) + + expect(requestStub.calledOnce).to.equal(true) + const [eventName, payload] = requestStub.firstCall.args + expect(eventName).to.equal('task:error') + expect(payload).to.have.property('clientId', 'client-1') + expect(payload).to.have.property('taskId', 'task-err') + expect(payload).to.have.property('projectPath', '/proj') + expect(payload.error).to.have.property('message') + }) + + it('emits task:error for unknown thrown values (non-Error)', () => { + handleExecutorTerminalError({ + clientId: 'client-1', + error: 'plain string', + log: (msg: string) => logCalls.push(msg), + projectPath: '/proj', + taskId: 'task-str', + transport, + }) + + expect(requestStub.calledOnce).to.equal(true) + expect(requestStub.firstCall.args[0]).to.equal('task:error') + }) + + it('swallows transport.request throw, never propagates', () => { + requestStub.throws(new Error('socket gone')) + + let thrown: unknown + try { + handleExecutorTerminalError({ + clientId: 'client-1', + error: new Error('inner'), + log: (msg: string) => logCalls.push(msg), + projectPath: '/proj', + taskId: 'task-2', + transport, + }) + } catch (error) { + thrown = error + } + + expect(thrown).to.equal(undefined) + expect(logCalls.some((l) => l.includes('task-2') && l.toLowerCase().includes('send failed'))).to.equal(true) + }) + + it('does NOT swallow non-cancel errors before logging — task:error path still logs the original error', () => { + handleExecutorTerminalError({ + clientId: 'client-1', + error: new Error('something broke'), + log: (msg: string) => logCalls.push(msg), + projectPath: '/proj', + taskId: 'task-log', + transport, + }) + + expect(logCalls.some((l) => l.includes('task:error') && l.includes('task-log'))).to.equal(true) + }) +}) diff --git a/test/unit/infra/daemon/agent-pool.test.ts b/test/unit/infra/daemon/agent-pool.test.ts index 9d3db5e87..fcbe51716 100644 --- a/test/unit/infra/daemon/agent-pool.test.ts +++ b/test/unit/infra/daemon/agent-pool.test.ts @@ -295,6 +295,42 @@ describe('AgentPool', () => { }) }) + describe('cancelQueuedTask (T1.3)', () => { + it('removes a queued task and returns true', async () => { + const transportServer = makeStubTransportServer() + const {pool} = createPool({maxConcurrentTasks: 1, transportServer}) + + await pool.submitTask(makeTask({projectPath: '/app', taskId: 't1'})) + await pool.submitTask(makeTask({projectPath: '/app', taskId: 't2-queued'})) + + const removed = pool.cancelQueuedTask('t2-queued') + expect(removed).to.equal(true) + + // Draining the queue now should not dispatch t2 (it was removed) + pool.notifyTaskCompleted('/app') + const dispatchedTaskIds = transportServer.sendTo.getCalls() + .filter((c) => c.args[1] === 'task:execute') + .map((c) => (c.args[2] as {taskId: string}).taskId) + expect(dispatchedTaskIds).to.not.include('t2-queued') + }) + + it('returns false when the task is not in any queue', () => { + const {pool} = createPool() + + const removed = pool.cancelQueuedTask('unknown') + expect(removed).to.equal(false) + }) + + it('returns false for a task that is currently in flight (not queued)', async () => { + const {pool} = createPool({maxConcurrentTasks: 1}) + + await pool.submitTask(makeTask({projectPath: '/app', taskId: 't-running'})) + + const removed = pool.cancelQueuedTask('t-running') + expect(removed).to.equal(false) + }) + }) + describe('pool capacity', () => { it('should return pool_full error when pool is full and new project needs agent', async () => { const {pool} = createPool({maxSize: 2}) diff --git a/test/unit/infra/process/connection-coordinator.test.ts b/test/unit/infra/process/connection-coordinator.test.ts index ac22d8b61..3d8cfe534 100644 --- a/test/unit/infra/process/connection-coordinator.test.ts +++ b/test/unit/infra/process/connection-coordinator.test.ts @@ -113,6 +113,7 @@ function makeStubClientManager(sandbox: SinonSandbox): IClientManager & { function makeStubAgentPool(sandbox: SinonSandbox): IAgentPool & {handleAgentDisconnected: SinonStub} { return { + cancelQueuedTask: sandbox.stub().returns(false), getEntries: sandbox.stub().returns([]), getSize: sandbox.stub().returns(0), handleAgentDisconnected: sandbox.stub(), diff --git a/test/unit/infra/process/task-router-accumulator.test.ts b/test/unit/infra/process/task-router-accumulator.test.ts index cf0ded430..ec7ca181c 100644 --- a/test/unit/infra/process/task-router-accumulator.test.ts +++ b/test/unit/infra/process/task-router-accumulator.test.ts @@ -61,6 +61,7 @@ function makeStubTransportServer(sandbox: SinonSandbox) { function makeStubAgentPool(sandbox: SinonSandbox): IAgentPool { return { + cancelQueuedTask: sandbox.stub().returns(false), getEntries: sandbox.stub().returns([]), getSize: sandbox.stub().returns(0), handleAgentDisconnected: sandbox.stub(), diff --git a/test/unit/infra/process/task-router.test.ts b/test/unit/infra/process/task-router.test.ts index 0a9019aa6..45c8e6d13 100644 --- a/test/unit/infra/process/task-router.test.ts +++ b/test/unit/infra/process/task-router.test.ts @@ -74,10 +74,12 @@ function makeStubTransportServer(sandbox: SinonSandbox) { } function makeStubAgentPool(sandbox: SinonSandbox): IAgentPool & { + cancelQueuedTask: SinonStub notifyTaskCompleted: SinonStub submitTask: SinonStub } { return { + cancelQueuedTask: sandbox.stub().returns(false), getEntries: sandbox.stub().returns([]), getSize: sandbox.stub().returns(0), handleAgentDisconnected: sandbox.stub(), @@ -768,6 +770,14 @@ describe('TaskRouter', () => { )).to.be.true }) + it('should notify agentPool on task:cancelled so the project queue drains (T1.3 queue-advance)', () => { + const handler = transportHelper.requestHandlers.get(TransportTaskEventNames.CANCELLED) + + handler!({taskId}, 'agent-1') + + expect(agentPool.notifyTaskCompleted.calledWith('/app')).to.be.true + }) + it('should remove task from active tasks after completion', () => { const handler = transportHelper.requestHandlers.get(TransportTaskEventNames.COMPLETED) @@ -840,6 +850,57 @@ describe('TaskRouter', () => { expect(result).to.deep.equal({error: 'Task not found', success: false}) }) + + describe('queued task cancellation (T1.3)', () => { + it('cancels a queued task directly via agentPool, broadcasts cancelled, never forwards to agent', () => { + agentPool.cancelQueuedTask.returns(true) + const handler = transportHelper.requestHandlers.get(TransportTaskEventNames.CANCEL) + + const result = handler!({taskId}, 'client-1') + + expect(result).to.deep.equal({success: true}) + expect(agentPool.cancelQueuedTask.calledOnceWithExactly(taskId)).to.equal(true) + + // Agent must NOT receive the forwarded cancel + const forwardedToAgent = (transportHelper.transport.sendTo as SinonStub).getCalls().some( + (c) => c.args[0] === 'agent-1' && c.args[1] === TransportTaskEventNames.CANCEL, + ) + expect(forwardedToAgent).to.equal(false) + + // Client receives task:cancelled directly from the daemon + expect((transportHelper.transport.sendTo as SinonStub).calledWith( + 'client-1', + TransportTaskEventNames.CANCELLED, + {taskId}, + )).to.be.true + + // Task removed from in-memory map + expect(router.getTasksForProject('/app')).to.have.lengthOf(0) + }) + + it('does NOT call agentPool.notifyTaskCompleted for queued cancel (the task never occupied a pool slot)', () => { + agentPool.cancelQueuedTask.returns(true) + const handler = transportHelper.requestHandlers.get(TransportTaskEventNames.CANCEL) + + handler!({taskId}, 'client-1') + + expect(agentPool.notifyTaskCompleted.called).to.equal(false) + }) + + it('falls through to forward-to-agent when cancelQueuedTask returns false (task is mid-execution)', () => { + agentPool.cancelQueuedTask.returns(false) + const handler = transportHelper.requestHandlers.get(TransportTaskEventNames.CANCEL) + + const result = handler!({taskId}, 'client-1') + + expect(result).to.deep.equal({success: true}) + expect((transportHelper.transport.sendTo as SinonStub).calledWith( + 'agent-1', + TransportTaskEventNames.CANCEL, + {taskId}, + )).to.be.true + }) + }) }) // ========================================================================== diff --git a/test/unit/infra/process/transport-handlers.test.ts b/test/unit/infra/process/transport-handlers.test.ts index 0020472c8..78525fb26 100644 --- a/test/unit/infra/process/transport-handlers.test.ts +++ b/test/unit/infra/process/transport-handlers.test.ts @@ -69,6 +69,7 @@ describe('TransportHandlers', () => { } mockAgentPool = { + cancelQueuedTask: sandbox.stub().returns(false), getEntries: sandbox.stub().returns([]), getSize: sandbox.stub().returns(0), handleAgentDisconnected: sandbox.stub(), From b558bad9cdbf54ff6d24fc1c279b41205495a4f7 Mon Sep 17 00:00:00 2001 From: Bao Ha Date: Wed, 13 May 2026 07:18:52 +0700 Subject: [PATCH 04/21] feat: [ENG-2778] integration test for end-to-end cancel pipeline MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - New test/integration/cancel-pipeline.test.ts covers the 5 T1.4 scenarios: cancel running, cancel running with queued, cancel queued, idempotent double-cancel, follow-up after cancel. - Harness uses a real SocketIOTransportServer, a real TaskRouter, and a stub IAgentPool backed by a real ProjectTaskQueue so the queue/drain logic is exercised end-to-end. A TransportClient acts as the mock agent and honours per-task behaviors (wait-for-cancel or auto-complete) so the test driver controls when each task ends. - History persistence is asserted via an ITaskLifecycleHook recorder rather than a real FileCurateLogStore — the recorder is the same hook interface CurateLogHandler implements in production to write status='cancelled' to history. The file-write itself stays covered by file-curate-log-store unit tests. - Suite stays in-memory; waitFor polls with a bounded 1500ms deadline. Whole file finishes in ~210ms with no hanging promises. --- test/integration/cancel-pipeline.test.ts | 457 +++++++++++++++++++++++ 1 file changed, 457 insertions(+) create mode 100644 test/integration/cancel-pipeline.test.ts diff --git a/test/integration/cancel-pipeline.test.ts b/test/integration/cancel-pipeline.test.ts new file mode 100644 index 000000000..35f38ac9a --- /dev/null +++ b/test/integration/cancel-pipeline.test.ts @@ -0,0 +1,457 @@ +import {TransportClient} from '@campfirein/brv-transport-client' +import {expect} from 'chai' +import {randomUUID} from 'node:crypto' + +import type {TaskExecute} from '../../src/server/core/domain/transport/schemas.js' +import type {TaskInfo} from '../../src/server/core/domain/transport/task-info.js' +import type {IAgentPool, SubmitTaskResult} from '../../src/server/core/interfaces/agent/i-agent-pool.js' +import type {ITaskLifecycleHook} from '../../src/server/core/interfaces/process/i-task-lifecycle-hook.js' +import type {IProjectRegistry} from '../../src/server/core/interfaces/project/i-project-registry.js' +import type {IProjectRouter} from '../../src/server/core/interfaces/routing/i-project-router.js' + +import {ProjectInfo} from '../../src/server/core/domain/project/project-info.js' +import {ProjectTaskQueue} from '../../src/server/infra/daemon/project-task-queue.js' +import {TaskRouter} from '../../src/server/infra/process/task-router.js' +import {SocketIOTransportServer} from '../../src/server/infra/transport/socket-io-transport-server.js' + +const PORT = 9802 +const delay = (ms: number): Promise => new Promise((resolve) => { setTimeout(resolve, ms) }) + +/** + * Wait for a predicate to become true, polling at 5ms intervals. + * Throws after `timeoutMs` to keep tests bounded. + */ +async function waitFor(check: () => T | undefined, label: string, timeoutMs = 1500): Promise { + const deadline = Date.now() + timeoutMs + while (Date.now() < deadline) { + const got = check() + if (got !== undefined) return got + // eslint-disable-next-line no-await-in-loop + await delay(5) + } + + throw new Error(`waitFor(${label}) timed out after ${timeoutMs}ms`) +} + +function makeProjectInfo(projectPath: string): ProjectInfo { + return new ProjectInfo({ + projectPath, + registeredAt: Date.now(), + sanitizedPath: projectPath.replaceAll('/', '_'), + storagePath: `/data${projectPath}`, + }) +} + +function makeProjectRegistry(): IProjectRegistry { + return { + get: (path: string) => makeProjectInfo(path), + getAll: () => new Map(), + register: (path: string) => makeProjectInfo(path), + unregister: () => true, + } +} + +function makeProjectRouter(): IProjectRouter { + return { + addToProjectRoom() {}, + broadcastToProject() {}, + getProjectMembers: () => [], + removeFromProjectRoom() {}, + } +} + +type StubPool = IAgentPool & { + resetActive(): void +} + +/** + * Stub IAgentPool backed by a real ProjectTaskQueue so the test exercises + * the actual queue logic. Forwards dispatched tasks to the connected mockAgent + * via the real transport server. + */ +function makeQueueBackedAgentPool( + server: SocketIOTransportServer, + getMockAgentClientId: () => string | undefined, + options?: {maxConcurrentTasksPerProject?: number}, +): StubPool { + const queue = new ProjectTaskQueue() + const active = new Map() + const maxConcurrent = options?.maxConcurrentTasksPerProject ?? 1 + + const dispatch = (task: TaskExecute): boolean => { + const id = getMockAgentClientId() + if (id === undefined) return false + server.sendTo(id, 'task:execute', task) + return true + } + + return { + cancelQueuedTask(taskId: string): boolean { + return queue.cancel(taskId) + }, + getEntries() { + return [] + }, + getSize() { + return active.size + }, + handleAgentDisconnected() {}, + hasAgent() { + return true + }, + markIdle() {}, + notifyTaskCompleted(projectPath: string) { + const cur = active.get(projectPath) ?? 0 + active.set(projectPath, Math.max(0, cur - 1)) + while ((active.get(projectPath) ?? 0) < maxConcurrent) { + const next = queue.dequeue(projectPath) + if (!next) break + active.set(projectPath, (active.get(projectPath) ?? 0) + 1) + if (!dispatch(next)) break + } + }, + resetActive() { + active.clear() + queue.clear() + }, + async shutdown() {}, + async submitTask(task: TaskExecute): Promise { + const projectKey = task.projectPath ?? '__default__' + const cur = active.get(projectKey) ?? 0 + if (cur >= maxConcurrent) { + queue.enqueue(projectKey, task) + return {success: true} + } + + if (!dispatch(task)) { + return {message: 'No agent connected', reason: 'create_failed', success: false} + } + + active.set(projectKey, cur + 1) + return {success: true} + }, + } +} + +type TaskBehavior = 'auto-complete' | 'wait-for-cancel' + +/** + * Bind a mockAgent to its lifecycle: + * - on task:execute, immediately emit task:started, then wait for either + * a cancel signal (default behavior) or an auto-complete timer + * (override per taskId via behaviors map). + * - on task:cancel, abort the in-flight controller for that task. The + * task body resolves on abort and emits task:cancelled (simulating the + * T1.1 cancel listener inside a real agent). + */ +function bindMockAgent(mockAgent: TransportClient, behaviors: Map) { + const inFlight = new Map() + const completed = new Set() + + mockAgent.on('task:execute', async (data: unknown) => { + const task = data as TaskExecute + await mockAgent.requestWithAck('task:started', {taskId: task.taskId}) + + const behavior = behaviors.get(task.taskId) ?? 'wait-for-cancel' + + if (behavior === 'auto-complete') { + await delay(15) + await mockAgent.requestWithAck('task:completed', { + projectPath: task.projectPath, + result: 'auto-complete result', + taskId: task.taskId, + }) + completed.add(task.taskId) + return + } + + const controller = new AbortController() + inFlight.set(task.taskId, controller) + + await new Promise((resolve) => { + controller.signal.addEventListener('abort', () => resolve(), {once: true}) + }) + inFlight.delete(task.taskId) + + await mockAgent.requestWithAck('task:cancelled', {taskId: task.taskId}) + completed.add(task.taskId) + }) + + mockAgent.on('task:cancel', (data: unknown) => { + const {taskId} = data as {taskId: string} + inFlight.get(taskId)?.abort() + }) + + return { + completed, + isInFlight(taskId: string): boolean { + return inFlight.has(taskId) + }, + } +} + +type HookCall = + | {errorMessage: string; kind: 'error'; taskId: string} + | {kind: 'cancelled'; taskId: string} + | {kind: 'completed'; result: string; taskId: string} + +function makeRecordingHook(): {calls: HookCall[]; hook: ITaskLifecycleHook} { + const calls: HookCall[] = [] + const hook: ITaskLifecycleHook = { + async onTaskCancelled(taskId: string, _task: TaskInfo) { + calls.push({kind: 'cancelled', taskId}) + }, + async onTaskCompleted(taskId: string, result: string, _task: TaskInfo) { + calls.push({kind: 'completed', result, taskId}) + }, + async onTaskError(taskId: string, errorMessage: string, _task: TaskInfo) { + calls.push({errorMessage, kind: 'error', taskId}) + }, + } + return {calls, hook} +} + +describe('Cancel pipeline (T1.4 integration)', () => { + let server: SocketIOTransportServer + let router: TaskRouter + let agentPool: StubPool + let mockAgent: TransportClient + let agentClientId: string | undefined + let client: TransportClient + let behaviors: Map + let agentBindings: ReturnType + let hookRecord: ReturnType + const projectPath = '/proj/cancel-pipeline-test' + + before(() => { + process.env.BRV_SESSION_LOG = '/dev/null' + }) + + after(() => { + delete process.env.BRV_SESSION_LOG + }) + + beforeEach(async () => { + server = new SocketIOTransportServer() + await server.start(PORT) + + agentClientId = undefined + server.onConnection((clientId) => { + if (!agentClientId) agentClientId = clientId + }) + + agentPool = makeQueueBackedAgentPool(server, () => agentClientId) + + hookRecord = makeRecordingHook() + + router = new TaskRouter({ + agentPool, + getAgentForProject: () => agentClientId, + lifecycleHooks: [hookRecord.hook], + projectRegistry: makeProjectRegistry(), + projectRouter: makeProjectRouter(), + transport: server, + }) + router.setup() + + mockAgent = new TransportClient() + await mockAgent.connect(`http://127.0.0.1:${PORT}`) + behaviors = new Map() + agentBindings = bindMockAgent(mockAgent, behaviors) + + client = new TransportClient() + await client.connect(`http://127.0.0.1:${PORT}`) + }) + + afterEach(async () => { + if (client?.getState() === 'connected') await client.disconnect() + if (mockAgent?.getState() === 'connected') await mockAgent.disconnect() + if (server?.isRunning()) { + router?.clearTasks?.() + await server.stop() + } + }) + + type TerminalRecord = {kind: 'cancelled' | 'completed' | 'error'; taskId: string} + + function captureTerminalEvents(c: TransportClient): TerminalRecord[] { + const records: TerminalRecord[] = [] + c.on('task:cancelled', (data: unknown) => { + records.push({kind: 'cancelled', taskId: (data as {taskId: string}).taskId}) + }) + c.on('task:completed', (data: unknown) => { + records.push({kind: 'completed', taskId: (data as {taskId: string}).taskId}) + }) + c.on('task:error', (data: unknown) => { + records.push({kind: 'error', taskId: (data as {taskId: string}).taskId}) + }) + return records + } + + function captureStartedEvents(c: TransportClient): string[] { + const ids: string[] = [] + c.on('task:started', (data: unknown) => { + ids.push((data as {taskId: string}).taskId) + }) + return ids + } + + it('scenario 1 — cancel running task; agent stays alive', async () => { + const terminal = captureTerminalEvents(client) + const started = captureStartedEvents(client) + + const taskId = randomUUID() + const agentIdBefore = agentClientId + + await client.requestWithAck('task:create', {content: 'slow', projectPath, taskId, type: 'curate'}) + await waitFor(() => (started.includes(taskId) ? true : undefined), 'task:started') + + const cancelResult = await client.requestWithAck<{success: boolean}>('task:cancel', {taskId}) + expect(cancelResult).to.deep.equal({success: true}) + + const cancelled = await waitFor( + () => terminal.find((t) => t.kind === 'cancelled' && t.taskId === taskId), + 'task:cancelled', + ) + expect(cancelled).to.deep.equal({kind: 'cancelled', taskId}) + + // No other terminal events for this task + const otherForTask = terminal.filter((t) => t.taskId === taskId && t.kind !== 'cancelled') + expect(otherForTask).to.have.length(0) + + // Lifecycle hook fired onTaskCancelled (proxy for persisted history status: 'cancelled') + expect(hookRecord.calls.some((c) => c.kind === 'cancelled' && c.taskId === taskId)).to.equal(true) + + // Agent (mock) is still alive — same socket id, still connected + expect(mockAgent.getState()).to.equal('connected') + expect(agentClientId).to.equal(agentIdBefore) + expect(mockAgent.getState()).to.equal('connected') + }) + + it('scenario 2 — cancel running task; queued task auto-starts', async () => { + const started = captureStartedEvents(client) + const terminal = captureTerminalEvents(client) + + const taskA = randomUUID() + const taskB = randomUUID() + behaviors.set(taskB, 'auto-complete') + + await client.requestWithAck('task:create', {content: 'a', projectPath, taskId: taskA, type: 'curate'}) + await waitFor(() => (started.includes(taskA) ? true : undefined), 'A started') + + await client.requestWithAck('task:create', {content: 'b', projectPath, taskId: taskB, type: 'curate'}) + // Confirm B is queued, not started yet + expect(started.includes(taskB)).to.equal(false) + + await client.requestWithAck<{success: boolean}>('task:cancel', {taskId: taskA}) + await waitFor( + () => terminal.find((t) => t.kind === 'cancelled' && t.taskId === taskA), + 'A cancelled', + ) + + // B should auto-dispatch after the queue drain + await waitFor(() => (started.includes(taskB) ? true : undefined), 'B started after A cancelled') + + // B should reach completion (auto-complete behavior) + await waitFor( + () => terminal.find((t) => t.kind === 'completed' && t.taskId === taskB), + 'B completed', + ) + }) + + it('scenario 3 — cancel queued task before it runs; agent never sees it', async () => { + const started = captureStartedEvents(client) + const terminal = captureTerminalEvents(client) + const cancelsSeenByAgent: string[] = [] + mockAgent.on('task:cancel', (data: unknown) => { + cancelsSeenByAgent.push((data as {taskId: string}).taskId) + }) + + const taskA = randomUUID() + const taskB = randomUUID() + + await client.requestWithAck('task:create', {content: 'a', projectPath, taskId: taskA, type: 'curate'}) + await waitFor(() => (started.includes(taskA) ? true : undefined), 'A started') + + await client.requestWithAck('task:create', {content: 'b', projectPath, taskId: taskB, type: 'curate'}) + expect(started.includes(taskB)).to.equal(false) + + await client.requestWithAck<{success: boolean}>('task:cancel', {taskId: taskB}) + + // B is cancelled from the daemon directly; emits task:cancelled + const cancelledForB = await waitFor( + () => terminal.find((t) => t.kind === 'cancelled' && t.taskId === taskB), + 'B cancelled (queued)', + ) + expect(cancelledForB).to.deep.equal({kind: 'cancelled', taskId: taskB}) + + // Agent never received task:cancel for B (only A may be cancelled later) + expect(cancelsSeenByAgent).to.not.include(taskB) + + // B never reached task:started + expect(started.includes(taskB)).to.equal(false) + + // A continues to run unaffected + expect(agentBindings.isInFlight(taskA)).to.equal(true) + }) + + it('scenario 4 — idempotent double-cancel emits exactly one terminal event', async () => { + const terminal = captureTerminalEvents(client) + const started = captureStartedEvents(client) + + const taskId = randomUUID() + await client.requestWithAck('task:create', {content: 'slow', projectPath, taskId, type: 'curate'}) + await waitFor(() => (started.includes(taskId) ? true : undefined), 'started') + + const [resA, resB] = await Promise.all([ + client.requestWithAck<{success: boolean}>('task:cancel', {taskId}), + client.requestWithAck<{success: boolean}>('task:cancel', {taskId}), + ]) + // At least one returns success: true; the other may return success: false + // (router reports "Task not found" once the task moves to completed). + // Either order is acceptable. + expect([resA.success, resB.success].filter(Boolean).length).to.be.greaterThanOrEqual(1) + + await waitFor( + () => terminal.find((t) => t.kind === 'cancelled' && t.taskId === taskId), + 'cancelled', + ) + + // Drain anything in flight + await delay(50) + const terminalsForTask = terminal.filter((t) => t.taskId === taskId) + expect(terminalsForTask).to.have.length(1) + expect(terminalsForTask[0].kind).to.equal('cancelled') + }) + + it('scenario 5 — follow-up task succeeds after cancel; agent stayed warm', async () => { + const started = captureStartedEvents(client) + const terminal = captureTerminalEvents(client) + + // First task: cancel + const taskA = randomUUID() + const agentIdBefore = agentClientId + await client.requestWithAck('task:create', {content: 'slow', projectPath, taskId: taskA, type: 'curate'}) + await waitFor(() => (started.includes(taskA) ? true : undefined), 'A started') + await client.requestWithAck<{success: boolean}>('task:cancel', {taskId: taskA}) + await waitFor( + () => terminal.find((t) => t.kind === 'cancelled' && t.taskId === taskA), + 'A cancelled', + ) + + // Follow-up task: auto-complete + const taskC = randomUUID() + behaviors.set(taskC, 'auto-complete') + await client.requestWithAck('task:create', {content: 'follow-up', projectPath, taskId: taskC, type: 'curate'}) + + await waitFor(() => (started.includes(taskC) ? true : undefined), 'C started') + await waitFor( + () => terminal.find((t) => t.kind === 'completed' && t.taskId === taskC), + 'C completed', + ) + + // Same agent socket throughout (no fork-recycle) + expect(agentClientId).to.equal(agentIdBefore) + expect(mockAgent.getState()).to.equal('connected') + }) +}) From af7488dcb1cccbb67f827493492b0df3113cb448 Mon Sep 17 00:00:00 2001 From: Bao Ha Date: Wed, 13 May 2026 07:30:53 +0700 Subject: [PATCH 05/21] feat: [ENG-2779] shared CLI helper that emits the cancel request MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - New src/oclif/lib/cancel-task.ts exposes runCancelTask({client, command, format, log, taskId}) — the only place in the CLI where the task:cancel transport event name and request shape appear. T2.2-T2.5 per-command --cancel flags will call it. - Text format prints "Cancelled " on success, "Failed to cancel : " on daemon-reported failure, with a generic fallback when the daemon omits the reason. - JSON format reuses writeJsonResponse so the payload shape stays consistent with other commands' --format json output. The caller's command name is stamped on the envelope so curate/query/dream stay greppable. - Returns boolean so the per-command caller can decide on exit code. - 7 unit tests cover both formats, success and failure paths, transport payload shape, and the missing-error-reason fallback. --- src/oclif/lib/cancel-task.ts | 62 ++++++++++ test/unit/oclif/lib/cancel-task.test.ts | 153 ++++++++++++++++++++++++ 2 files changed, 215 insertions(+) create mode 100644 src/oclif/lib/cancel-task.ts create mode 100644 test/unit/oclif/lib/cancel-task.test.ts diff --git a/src/oclif/lib/cancel-task.ts b/src/oclif/lib/cancel-task.ts new file mode 100644 index 000000000..8561533a1 --- /dev/null +++ b/src/oclif/lib/cancel-task.ts @@ -0,0 +1,62 @@ +/** + * Shared CLI helper that emits one task:cancel request and surfaces the + * response either as a plain text line or as the project's standard JSON + * envelope. Consumed by the curate, query, and dream commands' `--cancel` + * flag — the only place in the CLI where the cancel transport event name + * and request shape appear. + * + * Lives in the oclif layer only; do not export beyond it. + */ + +import type {ITransportClient} from '@campfirein/brv-transport-client' + +import type {TaskCancelRequest, TaskCancelResponse} from '../../shared/transport/events/task-events.js' + +import {TaskEvents} from '../../shared/transport/events/task-events.js' +import {writeJsonResponse} from './json-response.js' + +export type RunCancelTaskOptions = { + /** Pre-connected transport client (caller owns the lifecycle / retry wrapper). */ + client: ITransportClient + /** Name of the invoking CLI command — stamped on the JSON envelope. */ + command: string + /** Output format. JSON writes the project's standard envelope to stdout; text logs a single line. */ + format: 'json' | 'text' + /** Callback for text-mode output. Caller decides whether this goes to stdout, stderr, or oclif. */ + log: (msg: string) => void + /** Task to cancel. */ + taskId: string +} + +const UNKNOWN_ERROR = 'unknown error' + +/** + * Send the cancel request, format the response, return whether the cancel + * succeeded so the caller can decide on exit code. Does not throw on a + * daemon-reported failure — only on transport-level errors propagated by + * `client.requestWithAck`. + */ +export async function runCancelTask(options: RunCancelTaskOptions): Promise { + const {client, command, format, log, taskId} = options + + const response = await client.requestWithAck(TaskEvents.CANCEL, {taskId}) + + if (format === 'json') { + writeJsonResponse({ + command, + data: response.success + ? {status: 'cancelled', taskId} + : {error: response.error ?? UNKNOWN_ERROR, status: 'error', taskId}, + success: response.success, + }) + return response.success + } + + if (response.success) { + log(`Cancelled ${taskId}`) + } else { + log(`Failed to cancel ${taskId}: ${response.error ?? UNKNOWN_ERROR}`) + } + + return response.success +} diff --git a/test/unit/oclif/lib/cancel-task.test.ts b/test/unit/oclif/lib/cancel-task.test.ts new file mode 100644 index 000000000..f392dd096 --- /dev/null +++ b/test/unit/oclif/lib/cancel-task.test.ts @@ -0,0 +1,153 @@ +import type {ITransportClient} from '@campfirein/brv-transport-client' + +import {expect} from 'chai' +import {createSandbox, SinonSandbox, SinonStub} from 'sinon' + +import type {TaskCancelResponse} from '../../../../src/shared/transport/events/task-events.js' + +import {runCancelTask} from '../../../../src/oclif/lib/cancel-task.js' + +describe('runCancelTask', () => { + let sandbox: SinonSandbox + let requestStub: SinonStub + let client: Pick + let logCalls: string[] + let stdoutWrites: string[] + let stdoutWriteStub: SinonStub + + beforeEach(() => { + sandbox = createSandbox() + requestStub = sandbox.stub() + client = {requestWithAck: requestStub} as unknown as Pick + logCalls = [] + stdoutWrites = [] + stdoutWriteStub = sandbox.stub(process.stdout, 'write').callsFake((chunk: unknown) => { + stdoutWrites.push(String(chunk)) + return true + }) + }) + + afterEach(() => { + sandbox.restore() + }) + + it('emits task:cancel with the given taskId on the transport client', async () => { + requestStub.resolves({success: true} as TaskCancelResponse) + + await runCancelTask({ + client: client as ITransportClient, + command: 'curate', + format: 'text', + log: (msg: string) => logCalls.push(msg), + taskId: 'task-A', + }) + + expect(requestStub.calledOnce).to.equal(true) + expect(requestStub.firstCall.args[0]).to.equal('task:cancel') + expect(requestStub.firstCall.args[1]).to.deep.equal({taskId: 'task-A'}) + }) + + it('returns true and prints "Cancelled " on success (text format)', async () => { + requestStub.resolves({success: true} as TaskCancelResponse) + + const result = await runCancelTask({ + client: client as ITransportClient, + command: 'curate', + format: 'text', + log: (msg: string) => logCalls.push(msg), + taskId: 'task-B', + }) + + expect(result).to.equal(true) + expect(logCalls).to.include('Cancelled task-B') + expect(stdoutWriteStub.called).to.equal(false) + }) + + it('returns false and prints "Failed to cancel : " on failure (text format)', async () => { + requestStub.resolves({error: 'Task not found', success: false} as TaskCancelResponse) + + const result = await runCancelTask({ + client: client as ITransportClient, + command: 'curate', + format: 'text', + log: (msg: string) => logCalls.push(msg), + taskId: 'task-X', + }) + + expect(result).to.equal(false) + expect(logCalls.some((l) => l.includes('Failed to cancel task-X') && l.includes('Task not found'))).to.equal(true) + }) + + it('writes JSON success payload via writeJsonResponse (JSON format)', async () => { + requestStub.resolves({success: true} as TaskCancelResponse) + + const result = await runCancelTask({ + client: client as ITransportClient, + command: 'curate', + format: 'json', + log: (msg: string) => logCalls.push(msg), + taskId: 'task-J', + }) + + expect(result).to.equal(true) + expect(logCalls).to.have.length(0) + + expect(stdoutWrites).to.have.length(1) + const payload = JSON.parse(stdoutWrites[0]) + expect(payload).to.include({command: 'curate', success: true}) + expect(payload.data).to.deep.include({status: 'cancelled', taskId: 'task-J'}) + expect(payload).to.have.property('timestamp') + }) + + it('writes JSON failure payload with error reason (JSON format)', async () => { + requestStub.resolves({error: 'Task not found', success: false} as TaskCancelResponse) + + const result = await runCancelTask({ + client: client as ITransportClient, + command: 'dream', + format: 'json', + log: (msg: string) => logCalls.push(msg), + taskId: 'task-K', + }) + + expect(result).to.equal(false) + + const payload = JSON.parse(stdoutWrites[0]) + expect(payload).to.include({command: 'dream', success: false}) + expect(payload.data).to.deep.include({error: 'Task not found', status: 'error', taskId: 'task-K'}) + }) + + it('falls back to a generic error message when the daemon omits one', async () => { + requestStub.resolves({success: false} as TaskCancelResponse) + + await runCancelTask({ + client: client as ITransportClient, + command: 'query', + format: 'text', + log: (msg: string) => logCalls.push(msg), + taskId: 'task-Y', + }) + + // The exact wording is implementation-defined; we just require the + // taskId, an error label, and that no "undefined" leaks into output. + const line = logCalls.find((l) => l.includes('task-Y')) + expect(line).to.exist + expect(line!.toLowerCase()).to.include('fail') + expect(line!).to.not.include('undefined') + }) + + it('passes the caller-supplied command verbatim into the JSON payload', async () => { + requestStub.resolves({success: true} as TaskCancelResponse) + + await runCancelTask({ + client: client as ITransportClient, + command: 'query', + format: 'json', + log() {}, + taskId: 'task-Q', + }) + + const payload = JSON.parse(stdoutWrites[0]) + expect(payload.command).to.equal('query') + }) +}) From 31306eaabb0e41d7760861f1ed83f7732863028b Mon Sep 17 00:00:00 2001 From: Bao Ha Date: Wed, 13 May 2026 09:05:02 +0700 Subject: [PATCH 06/21] feat: [ENG-2780] brv curate --cancel flag - New --cancel flag short-circuits the create flow: no provider validation, no task:create, no context required. Delegates to the shared runCancelTask helper from T2.1 and exits 1 on daemon-reported failure so scripts can rely on the exit code. - Mutually exclusive with --files, --folder, --detach (oclif's `exclusive`). --timeout is left allowed and silently ignored on the cancel branch; rejecting a harmless combo would surprise users. - Existing curate behavior is untouched. The cancel branch is added alongside the normal path with zero overlap on the create code. - 10 new tests in test/commands/curate.test.ts cover both formats, success and failure paths, mutex enforcement against each of the three exclusive flags, and the timeout-allowed case. --- src/oclif/commands/curate/index.ts | 34 +++++++ test/commands/curate.test.ts | 143 +++++++++++++++++++++++++++++ 2 files changed, 177 insertions(+) diff --git a/src/oclif/commands/curate/index.ts b/src/oclif/commands/curate/index.ts index 75ca8d12f..a5097ffe4 100644 --- a/src/oclif/commands/curate/index.ts +++ b/src/oclif/commands/curate/index.ts @@ -9,6 +9,7 @@ import {BRV_DIR, CONTEXT_TREE_DIR} from '../../../server/constants.js' import {ProviderConfigResponse, TransportStateEventNames} from '../../../server/core/domain/transport/index.js' import {extractCurateOperations} from '../../../server/utils/curate-result-parser.js' import {TaskEvents} from '../../../shared/transport/events/index.js' +import {runCancelTask} from '../../lib/cancel-task.js' import { type DaemonClientOptions, formatConnectionError, @@ -68,6 +69,10 @@ Bad examples: '<%= config.bin %> curate view --status completed --since 1h', ] public static flags = { + cancel: Flags.string({ + description: 'Cancel a running task by id. Short-circuits the create flow — no new task is created.', + exclusive: ['files', 'folder', 'detach'], + }), detach: Flags.boolean({ default: false, description: 'Queue task and exit without waiting for completion', @@ -110,6 +115,11 @@ Bad examples: } const format: 'json' | 'text' = flags.format ?? 'text' + if (rawFlags.cancel) { + await this.runCancelBranch(rawFlags.cancel, format) + return + } + if (!this.validateInput(args, flags, format)) return const resolvedContent = args.context?.trim() @@ -286,6 +296,30 @@ Bad examples: } } + private async runCancelBranch(taskId: string, format: 'json' | 'text'): Promise { + let success = false + try { + await withDaemonRetry( + async (client) => { + success = await runCancelTask({ + client, + command: 'curate', + format, + log: (msg) => this.log(msg), + taskId, + }) + }, + this.getDaemonClientOptions(), + ) + } catch (error) { + this.reportError(error, format) + this.exit(1) + return + } + + if (!success) this.exit(1) + } + private async submitTask(props: { client: ITransportClient content: string diff --git a/test/commands/curate.test.ts b/test/commands/curate.test.ts index c33841428..f967707b3 100644 --- a/test/commands/curate.test.ts +++ b/test/commands/curate.test.ts @@ -567,4 +567,147 @@ describe('Curate Command', () => { expect(loggedMessages.some((m) => m.includes('✓ Context curated successfully'))).to.be.true }) }) + + // ==================== --cancel flag (T2.2) ==================== + + describe('--cancel flag', () => { + // eslint-disable-next-line unicorn/consistent-function-scoping -- captures mockClient from outer beforeEach + function stubCancelResponse(response: {error?: string; success: boolean}): void { + ;(mockClient.requestWithAck as sinon.SinonStub).callsFake(async (event: string) => { + if (event === 'task:cancel') return response + // Provider config still answers if the command ever asks — but the cancel + // branch should never get here. Returning a config keeps the stub honest + // so the assertion below catches a missed short-circuit. + return {activeProvider: 'anthropic'} + }) + } + + it('short-circuits the create flow: emits task:cancel, never asks provider config or task:create', async () => { + stubCancelResponse({success: true}) + + await createCommand('--cancel', 'task-A').run() + + const requestStub = mockClient.requestWithAck as sinon.SinonStub + const eventNames = requestStub.getCalls().map((c) => c.args[0]) + expect(eventNames).to.deep.equal(['task:cancel']) + expect(requestStub.firstCall.args[1]).to.deep.equal({taskId: 'task-A'}) + }) + + it('prints "Cancelled " on success (text format)', async () => { + stubCancelResponse({success: true}) + + await createCommand('--cancel', 'task-B').run() + + expect(loggedMessages).to.include('Cancelled task-B') + }) + + it('prints a failure line including the daemon-provided reason (text format) and exits non-zero', async () => { + stubCancelResponse({error: 'Task not found', success: false}) + + let exitError: unknown + try { + await createCommand('--cancel', 'task-X').run() + } catch (error) { + exitError = error + } + + expect(loggedMessages.some((m) => m.includes('Failed to cancel task-X') && m.includes('Task not found'))).to.be.true + // oclif throws ExitError when this.exit(1) runs + expect(exitError).to.not.equal(undefined) + }) + + it('emits the project JSON envelope when --format json is given (success)', async () => { + stubCancelResponse({success: true}) + + await createJsonCommand('--cancel', 'task-J').run() + + const json = parseJsonOutput() + expect(json.command).to.equal('curate') + expect(json.success).to.equal(true) + expect(json.data).to.deep.include({status: 'cancelled', taskId: 'task-J'}) + }) + + it('emits the project JSON envelope when --format json is given (failure)', async () => { + stubCancelResponse({error: 'Task not found', success: false}) + + try { + await createJsonCommand('--cancel', 'task-K').run() + } catch { + // ExitError on non-zero exit + } + + const json = parseJsonOutput() + expect(json.command).to.equal('curate') + expect(json.success).to.equal(false) + expect(json.data).to.deep.include({error: 'Task not found', status: 'error', taskId: 'task-K'}) + }) + + it('does NOT call validateInput (no context required when --cancel is used)', async () => { + stubCancelResponse({success: true}) + + await createCommand('--cancel', 'task-C').run() + + // Without --cancel, missing context would log this hint. + expect(loggedMessages.some((m) => m.includes('Either a context argument'))).to.equal(false) + }) + + it('rejects --cancel together with --files (mutually exclusive)', async () => { + stubCancelResponse({success: true}) + + let parseError: unknown + try { + await createCommand('--cancel', 'task-Z', '--files', 'foo.ts').run() + } catch (error) { + parseError = error + } + + // oclif throws CLIError when exclusive flags are combined; no transport calls are made. + expect(parseError).to.not.equal(undefined) + expect((mockClient.requestWithAck as sinon.SinonStub).called).to.equal(false) + }) + + it('rejects --cancel together with --folder (mutually exclusive)', async () => { + stubCancelResponse({success: true}) + + let parseError: unknown + try { + await createCommand('--cancel', 'task-Z', '--folder', 'src/').run() + } catch (error) { + parseError = error + } + + expect(parseError).to.not.equal(undefined) + expect((mockClient.requestWithAck as sinon.SinonStub).called).to.equal(false) + }) + + it('rejects --cancel together with --detach (mutually exclusive)', async () => { + stubCancelResponse({success: true}) + + let parseError: unknown + try { + await createCommand('--cancel', 'task-Z', '--detach').run() + } catch (error) { + parseError = error + } + + expect(parseError).to.not.equal(undefined) + expect((mockClient.requestWithAck as sinon.SinonStub).called).to.equal(false) + }) + + it('allows --cancel alongside --timeout (timeout has no effect on the cancel branch)', async () => { + stubCancelResponse({success: true}) + + let parseError: unknown + try { + await createCommand('--cancel', 'task-T', '--timeout', '60').run() + } catch (error) { + parseError = error + } + + expect(parseError).to.equal(undefined) + const requestStub = mockClient.requestWithAck as sinon.SinonStub + const eventNames = requestStub.getCalls().map((c) => c.args[0]) + expect(eventNames).to.deep.equal(['task:cancel']) + }) + }) }) From 5c5411981baf4c60a2981d1dfda5a878dd7a740c Mon Sep 17 00:00:00 2001 From: Bao Ha Date: Wed, 13 May 2026 09:16:02 +0700 Subject: [PATCH 07/21] feat: [ENG-2781] brv query --cancel flag - Add --cancel flag that short-circuits the query flow through the shared runCancelTask helper from T2.1. - Relax the positional `query` argument to optional so --cancel can be used without a placeholder query string. A manual guard in run() preserves the existing missing-query error when neither input is given. - Reject " --cancel " as a mutually exclusive combination with a clear error message in both text and JSON formats, exiting non-zero without touching the transport. - Help text on both surfaces reflects the dual mode (run a query, or cancel by id). - 7 new tests cover both formats, success and failure paths, the positional+cancel conflict, and missing-both rejection. --- src/oclif/commands/query.ts | 62 +++++++++++++++++++-- test/commands/query.test.ts | 108 ++++++++++++++++++++++++++++++++++++ 2 files changed, 165 insertions(+), 5 deletions(-) diff --git a/src/oclif/commands/query.ts b/src/oclif/commands/query.ts index b2f4045a3..3fdd93cae 100644 --- a/src/oclif/commands/query.ts +++ b/src/oclif/commands/query.ts @@ -5,6 +5,7 @@ import {randomUUID} from 'node:crypto' import {type ProviderConfigResponse, TransportStateEventNames} from '../../server/core/domain/transport/schemas.js' import {TaskEvents} from '../../shared/transport/events/index.js' +import {runCancelTask} from '../lib/cancel-task.js' import { type DaemonClientOptions, formatConnectionError, @@ -25,8 +26,8 @@ type QueryFlags = { export default class Query extends Command { public static args = { query: Args.string({ - description: 'Natural language question about your codebase or project knowledge', - required: true, + description: 'Natural language question about your codebase or project knowledge (omit when using --cancel)', + required: false, }), } public static description = `Query and retrieve information from the context tree @@ -46,6 +47,9 @@ Bad: '<%= config.bin %> <%= command.id %> "How does auth work?" --format json', ] public static flags = { + cancel: Flags.string({ + description: 'Cancel a running task by id. Short-circuits the query flow — no new task is created.', + }), format: Flags.string({ default: 'text', description: 'Output format (text or json)', @@ -66,10 +70,21 @@ Bad: public async run(): Promise { const {args, flags: rawFlags} = await this.parse(Query) - const flags = rawFlags as QueryFlags + const flags = rawFlags as QueryFlags & {cancel?: string} const format = (flags.format ?? 'text') as 'json' | 'text' - if (!this.validateInput(args.query, format)) return + if (flags.cancel) { + if (args.query !== undefined && args.query.trim() !== '') { + this.reportCombinationError(format) + this.exit(1) + return + } + + await this.runCancelBranch(flags.cancel, format) + return + } + + if (!this.validateInput(args.query ?? '', format)) return let providerContext: ProviderErrorContext | undefined @@ -95,7 +110,7 @@ Bad: client, format, projectRoot, - query: args.query, + query: args.query ?? '', timeoutMs: (flags.timeout ?? DEFAULT_TIMEOUT_SECONDS) * 1000, worktreeRoot, }) @@ -114,6 +129,19 @@ Bad: } } + private reportCombinationError(format: 'json' | 'text'): void { + const message = 'Provide either a query string or --cancel , not both.' + if (format === 'json') { + writeJsonResponse({ + command: 'query', + data: {message, status: 'error'}, + success: false, + }) + } else { + this.log(message) + } + } + private reportError(error: unknown, format: 'json' | 'text', providerContext?: ProviderErrorContext): void { const errorMessage = error instanceof Error ? error.message : 'Query failed' @@ -129,6 +157,30 @@ Bad: } } + private async runCancelBranch(taskId: string, format: 'json' | 'text'): Promise { + let success = false + try { + await withDaemonRetry( + async (client) => { + success = await runCancelTask({ + client, + command: 'query', + format, + log: (msg) => this.log(msg), + taskId, + }) + }, + this.getDaemonClientOptions(), + ) + } catch (error) { + this.reportError(error, format) + this.exit(1) + return + } + + if (!success) this.exit(1) + } + private async submitTask(props: { client: ITransportClient format: 'json' | 'text' diff --git a/test/commands/query.test.ts b/test/commands/query.test.ts index a4fbc866c..c9ff029c6 100644 --- a/test/commands/query.test.ts +++ b/test/commands/query.test.ts @@ -631,4 +631,112 @@ describe('Query Command', () => { expect(loggedMessages.some((m) => m.includes('done'))).to.be.true }) }) + + // ==================== --cancel flag (T2.3) ==================== + + describe('--cancel flag', () => { + // eslint-disable-next-line unicorn/consistent-function-scoping -- captures mockClient from outer beforeEach + function stubCancelResponse(response: {error?: string; success: boolean}): void { + ;(mockClient.requestWithAck as sinon.SinonStub).callsFake(async (event: string) => { + if (event === 'task:cancel') return response + return {activeProvider: 'anthropic'} + }) + } + + it('short-circuits the create flow: emits task:cancel only', async () => { + stubCancelResponse({success: true}) + + await createCommand('--cancel', 'task-A').run() + + const requestStub = mockClient.requestWithAck as sinon.SinonStub + const eventNames = requestStub.getCalls().map((c) => c.args[0]) + expect(eventNames).to.deep.equal(['task:cancel']) + expect(requestStub.firstCall.args[1]).to.deep.equal({taskId: 'task-A'}) + }) + + it('does not require the positional query argument when --cancel is set', async () => { + stubCancelResponse({success: true}) + + // No positional arg, no error + let parseError: unknown + try { + await createCommand('--cancel', 'task-B').run() + } catch (error) { + parseError = error + } + + expect(parseError).to.equal(undefined) + expect(loggedMessages).to.include('Cancelled task-B') + }) + + it('prints failure line with daemon-reported reason and exits non-zero (text)', async () => { + stubCancelResponse({error: 'Task not found', success: false}) + + let exitError: unknown + try { + await createCommand('--cancel', 'task-X').run() + } catch (error) { + exitError = error + } + + expect(loggedMessages.some((m) => m.includes('Failed to cancel task-X') && m.includes('Task not found'))).to.be.true + expect(exitError).to.not.equal(undefined) + }) + + it('emits the project JSON envelope (success)', async () => { + stubCancelResponse({success: true}) + + await createJsonCommand('--cancel', 'task-J').run() + + const [json] = parseJsonOutput() + expect(json.command).to.equal('query') + expect(json.success).to.equal(true) + expect(json.data).to.deep.include({status: 'cancelled', taskId: 'task-J'}) + }) + + it('emits the project JSON envelope (failure)', async () => { + stubCancelResponse({error: 'Task not found', success: false}) + + try { + await createJsonCommand('--cancel', 'task-K').run() + } catch { + // ExitError on non-zero exit + } + + const [json] = parseJsonOutput() + expect(json.command).to.equal('query') + expect(json.success).to.equal(false) + expect(json.data).to.deep.include({error: 'Task not found', status: 'error', taskId: 'task-K'}) + }) + + it('rejects positional query string together with --cancel (mutex)', async () => { + stubCancelResponse({success: true}) + + let exitError: unknown + try { + await createCommand('what is X', '--cancel', 'task-Z').run() + } catch (error) { + exitError = error + } + + // Either a clear logged error + non-zero exit, or just a thrown error. + const loggedErr = loggedMessages.some((m) => m.toLowerCase().includes('cancel') && m.toLowerCase().includes('query')) + expect(loggedErr || exitError !== undefined).to.equal(true) + // No transport call must be made when the combination is rejected. + expect((mockClient.requestWithAck as sinon.SinonStub).called).to.equal(false) + }) + + it('rejects missing both positional query and --cancel with the existing missing-query error', async () => { + let exitError: unknown + try { + await createCommand().run() + } catch (error) { + exitError = error + } + + // Missing-query message OR a thrown error from oclif arg validation. + const loggedMissing = loggedMessages.some((m) => m.includes('Query argument is required')) + expect(loggedMissing || exitError !== undefined).to.equal(true) + }) + }) }) From 9e423a396be9de41c82260f46e34bdd7a0855618 Mon Sep 17 00:00:00 2001 From: Bao Ha Date: Wed, 13 May 2026 09:26:07 +0700 Subject: [PATCH 08/21] feat: [ENG-2782] brv dream --cancel flag - Add --cancel flag that short-circuits the dream flow through the shared runCancelTask helper from T2.1. Branches BEFORE the --undo path so cancel never accidentally triggers a revert. - Mutually exclusive with --force, --undo, --detach (oclif's `exclusive`). --timeout is left allowed and silently ignored on the cancel branch. - Help text makes clear that cancel is a hard stop and not a revert; pair with `brv dream --undo` for rollback of completed/partial dreams. - 9 new tests cover both formats, success and failure paths, the mutex against each of the three exclusive flags, and the timeout-allowed case. --- src/oclif/commands/dream.ts | 34 ++++++++++ test/commands/dream.test.ts | 129 ++++++++++++++++++++++++++++++++++++ 2 files changed, 163 insertions(+) diff --git a/src/oclif/commands/dream.ts b/src/oclif/commands/dream.ts index b6f2acb4e..1f6a27b4c 100644 --- a/src/oclif/commands/dream.ts +++ b/src/oclif/commands/dream.ts @@ -22,6 +22,7 @@ import {FileCurateLogStore} from '../../server/infra/storage/file-curate-log-sto import {FileReviewBackupStore} from '../../server/infra/storage/file-review-backup-store.js' import {getProjectDataDir} from '../../server/utils/path-utils.js' import {TaskEvents} from '../../shared/transport/events/index.js' +import {runCancelTask} from '../lib/cancel-task.js' import { type DaemonClientOptions, formatConnectionError, @@ -83,6 +84,10 @@ export default class Dream extends Command { '<%= config.bin %> <%= command.id %> --format json', ] public static flags = { + cancel: Flags.string({ + description: 'Cancel a running dream task by id. Hard stop — does not revert any partial writes (use --undo for that). Short-circuits the dream flow.', + exclusive: ['force', 'undo', 'detach'], + }), detach: Flags.boolean({ default: false, description: 'Queue task and exit without waiting for completion', @@ -117,6 +122,11 @@ export default class Dream extends Command { const {flags: rawFlags} = await this.parse(Dream) const format = rawFlags.format === 'json' ? 'json' : 'text' + if (rawFlags.cancel) { + await this.runCancelBranch(rawFlags.cancel, format) + return + } + if (rawFlags.undo) { await this.runUndo(format) return @@ -181,6 +191,30 @@ export default class Dream extends Command { } } + private async runCancelBranch(taskId: string, format: 'json' | 'text'): Promise { + let success = false + try { + await withDaemonRetry( + async (client) => { + success = await runCancelTask({ + client, + command: 'dream', + format, + log: (msg) => this.log(msg), + taskId, + }) + }, + this.getDaemonClientOptions(), + ) + } catch (error) { + this.reportError(error, format) + this.exit(1) + return + } + + if (!success) this.exit(1) + } + private async runUndo(format: 'json' | 'text'): Promise { const projectRoot = resolveProject()?.projectRoot ?? process.cwd() // JSON mode: route sidecar warnings to a no-op so structured stdout diff --git a/test/commands/dream.test.ts b/test/commands/dream.test.ts index 282a3b912..3dcfd6cad 100644 --- a/test/commands/dream.test.ts +++ b/test/commands/dream.test.ts @@ -161,4 +161,133 @@ describe('Dream Command', () => { expect(mockClient.disconnect.calledOnce).to.be.true }) }) + + // ==================== --cancel flag (T2.4) ==================== + + describe('--cancel flag', () => { + // eslint-disable-next-line unicorn/consistent-function-scoping -- captures mockClient from outer beforeEach + function stubCancelResponse(response: {error?: string; success: boolean}): void { + ;(mockClient.requestWithAck as sinon.SinonStub).callsFake(async (event: string) => { + if (event === 'task:cancel') return response + return {activeProvider: 'anthropic'} + }) + } + + it('short-circuits the dream flow: emits task:cancel only, never starts a dream', async () => { + stubCancelResponse({success: true}) + + await createCommand('--cancel', 'task-A').run() + + const requestStub = mockClient.requestWithAck as sinon.SinonStub + const eventNames = requestStub.getCalls().map((c) => c.args[0]) + expect(eventNames).to.deep.equal(['task:cancel']) + expect(requestStub.firstCall.args[1]).to.deep.equal({taskId: 'task-A'}) + }) + + it('prints "Cancelled " on success (text format)', async () => { + stubCancelResponse({success: true}) + + await createCommand('--cancel', 'task-B').run() + + expect(loggedMessages).to.include('Cancelled task-B') + }) + + it('prints failure line with daemon-reported reason and exits non-zero (text)', async () => { + stubCancelResponse({error: 'Task not found', success: false}) + + let exitError: unknown + try { + await createCommand('--cancel', 'task-X').run() + } catch (error) { + exitError = error + } + + expect(loggedMessages.some((m) => m.includes('Failed to cancel task-X') && m.includes('Task not found'))).to.be.true + expect(exitError).to.not.equal(undefined) + }) + + it('emits the project JSON envelope (success)', async () => { + stubCancelResponse({success: true}) + + await createJsonCommand('--cancel', 'task-J').run() + + const json = parseJsonOutput() + expect(json.command).to.equal('dream') + expect(json.success).to.equal(true) + expect(json.data).to.deep.include({status: 'cancelled', taskId: 'task-J'}) + }) + + it('emits the project JSON envelope (failure)', async () => { + stubCancelResponse({error: 'Task not found', success: false}) + + try { + await createJsonCommand('--cancel', 'task-K').run() + } catch { + // ExitError + } + + const json = parseJsonOutput() + expect(json.command).to.equal('dream') + expect(json.success).to.equal(false) + expect(json.data).to.deep.include({error: 'Task not found', status: 'error', taskId: 'task-K'}) + }) + + it('rejects --cancel together with --force (mutually exclusive)', async () => { + stubCancelResponse({success: true}) + + let parseError: unknown + try { + await createCommand('--cancel', 'task-Z', '--force').run() + } catch (error) { + parseError = error + } + + expect(parseError).to.not.equal(undefined) + expect((mockClient.requestWithAck as sinon.SinonStub).called).to.equal(false) + }) + + it('rejects --cancel together with --undo (mutually exclusive)', async () => { + stubCancelResponse({success: true}) + + let parseError: unknown + try { + await createCommand('--cancel', 'task-Z', '--undo').run() + } catch (error) { + parseError = error + } + + expect(parseError).to.not.equal(undefined) + expect((mockClient.requestWithAck as sinon.SinonStub).called).to.equal(false) + }) + + it('rejects --cancel together with --detach (mutually exclusive)', async () => { + stubCancelResponse({success: true}) + + let parseError: unknown + try { + await createCommand('--cancel', 'task-Z', '--detach').run() + } catch (error) { + parseError = error + } + + expect(parseError).to.not.equal(undefined) + expect((mockClient.requestWithAck as sinon.SinonStub).called).to.equal(false) + }) + + it('allows --cancel alongside --timeout (timeout has no effect on the cancel branch)', async () => { + stubCancelResponse({success: true}) + + let parseError: unknown + try { + await createCommand('--cancel', 'task-T', '--timeout', '60').run() + } catch (error) { + parseError = error + } + + expect(parseError).to.equal(undefined) + const requestStub = mockClient.requestWithAck as sinon.SinonStub + const eventNames = requestStub.getCalls().map((c) => c.args[0]) + expect(eventNames).to.deep.equal(['task:cancel']) + }) + }) }) From 903441fbfefc055d75d6fc3cf6687c89135dd5b0 Mon Sep 17 00:00:00 2001 From: Bao Ha Date: Wed, 13 May 2026 09:44:10 +0700 Subject: [PATCH 09/21] feat: [ENG-2783] foreground Ctrl-C sends cancel before exit - waitForTaskCompletion installs a SIGINT handler for the duration of the wait. First Ctrl-C emits task:cancel for the current task and writes a "Cancelling task..." hint to stderr (kept off stdout so --format json output stays clean). Second Ctrl-C hard-exits with code 130 (conventional SIGINT exit) without re-emitting cancel. - The wait loop now subscribes to task:cancelled as a terminal event. Previously it only handled task:completed and task:error, which meant a remote --cancel or a Ctrl-C cancel left the foreground process hanging until the existing timeout fired. The new branch fires the optional onCancelled callback and resolves the promise. - The SIGINT handler is added to the same unsubscribers chain the existing cleanup() drains, so it is removed on every terminal path (completed, cancelled, error, disconnect, timeout). - 8 new unit tests in test/unit/oclif/lib/task-client-sigint.test.ts cover both terminal recognition and the SIGINT install/uninstall contract. --- src/oclif/lib/task-client.ts | 50 ++- .../unit/oclif/lib/task-client-sigint.test.ts | 305 ++++++++++++++++++ 2 files changed, 354 insertions(+), 1 deletion(-) create mode 100644 test/unit/oclif/lib/task-client-sigint.test.ts diff --git a/src/oclif/lib/task-client.ts b/src/oclif/lib/task-client.ts index 4f2cef3dd..f137540a0 100644 --- a/src/oclif/lib/task-client.ts +++ b/src/oclif/lib/task-client.ts @@ -93,6 +93,8 @@ export interface WaitForTaskOptions { command: string /** Output format */ format: 'json' | 'text' + /** Called on task:cancelled (terminal). Optional; defaults to a no-op so legacy callers stay compatible. */ + onCancelled?: (result: {taskId: string; toolCalls: ToolCallRecord[]}) => void /** Called on task:completed */ onCompleted: (result: TaskCompletionResult) => void /** Called on task:error */ @@ -161,6 +163,39 @@ export function formatToolDisplay(toolName: string, args: Record void { + let cancelRequested = false + + const onSigint = (): void => { + if (cancelRequested) { + // eslint-disable-next-line n/no-process-exit, unicorn/no-process-exit + process.exit(130) + return + } + + cancelRequested = true + process.stderr.write('\nCancelling task... (Ctrl-C again to force exit)\n') + // Fire-and-forget; we await the daemon's task:cancelled broadcast in the + // wait loop. The daemon may also respond with success/false on a stale id; + // we ignore that here because the wait loop times out or sees a terminal + // event either way. + deps.client.request(TaskEvents.CANCEL, {taskId: deps.taskId}) + } + + process.on('SIGINT', onSigint) + return () => process.off('SIGINT', onSigint) +} + /** * Wait for task completion by subscribing to all events (same as TUI useTaskSubscriptions). * In text mode: streams tool calls and thinking in real-time via log(). @@ -170,7 +205,7 @@ export function formatToolDisplay(toolName: string, args: Record void): Promise { - const {client, command, format, onCompleted, onError, onResponse, taskId, timeoutMs = DEFAULT_TIMEOUT_MS} = options + const {client, command, format, onCancelled, onCompleted, onError, onResponse, taskId, timeoutMs = DEFAULT_TIMEOUT_MS} = options const isText = format === 'text' return new Promise((resolve, reject) => { @@ -342,6 +377,18 @@ export function waitForTaskCompletion(options: WaitForTaskOptions, log: (msg: st } }), + // Task cancelled (terminal) — T2.5. The daemon broadcasts this in + // response to either a foreground Ctrl-C (handler below) or a remote + // --cancel from another terminal. Treated as a terminal event so the + // wait loop unblocks instead of hanging. + client.on<{taskId: string}>(TaskEvents.CANCELLED, (payload) => { + if (payload.taskId !== taskId || completed) return + completed = true + cleanup() + onCancelled?.({taskId, toolCalls}) + resolve() + }), + // Connection state monitoring client.onStateChange((state) => { if (completed) return @@ -371,6 +418,7 @@ export function waitForTaskCompletion(options: WaitForTaskOptions, log: (msg: st () => { if (disconnectTimer) clearTimeout(disconnectTimer) }, + installSigintCancel({client, taskId}), ] const cleanup = (): void => { diff --git a/test/unit/oclif/lib/task-client-sigint.test.ts b/test/unit/oclif/lib/task-client-sigint.test.ts new file mode 100644 index 000000000..18fee3938 --- /dev/null +++ b/test/unit/oclif/lib/task-client-sigint.test.ts @@ -0,0 +1,305 @@ +import type {ITransportClient} from '@campfirein/brv-transport-client' + +import {expect} from 'chai' +import {createSandbox, type SinonSandbox, type SinonStub} from 'sinon' + +import {waitForTaskCompletion} from '../../../../src/oclif/lib/task-client.js' + +type EventHandler = (data: unknown) => void + +/** + * Build a TransportClient stub that exposes: + * - eventHandlers map keyed by event name with the registered listener + * - on/onStateChange returning unsubscribe stubs (counted for cleanup checks) + * - request stub for verifying the cancel emission + */ +function makeStubClient(sandbox: SinonSandbox) { + const eventHandlers = new Map() + const unsubscribeStubs: SinonStub[] = [] + + const onStub = sandbox.stub().callsFake((event: string, handler: EventHandler) => { + eventHandlers.set(event, handler) + const unsub = sandbox.stub() + unsubscribeStubs.push(unsub) + return unsub + }) + + const stateUnsub = sandbox.stub() + const onStateChangeStub = sandbox.stub().returns(stateUnsub) + unsubscribeStubs.push(stateUnsub) + + const client = { + on: onStub, + onStateChange: onStateChangeStub, + request: sandbox.stub(), + requestWithAck: sandbox.stub(), + } as unknown as ITransportClient + + return { + client, + emit(event: string, data: unknown) { + eventHandlers.get(event)?.(data) + }, + onStub, + requestStub: client.request as SinonStub, + unsubscribeStubs, + } +} + +describe('waitForTaskCompletion — SIGINT cancel handling (T2.5)', () => { + let sandbox: SinonSandbox + let stderrWrites: string[] + let stderrStub: SinonStub + let exitStub: SinonStub + let originalSigintListeners: Array<(() => void) | NodeJS.SignalsListener> + + beforeEach(() => { + sandbox = createSandbox() + stderrWrites = [] + stderrStub = sandbox.stub(process.stderr, 'write').callsFake((chunk: unknown) => { + stderrWrites.push(String(chunk)) + return true + }) + exitStub = sandbox.stub(process, 'exit') + + // Park existing SIGINT listeners so the test runner doesn't fire on emit. + originalSigintListeners = process.listeners('SIGINT') as Array<(() => void) | NodeJS.SignalsListener> + process.removeAllListeners('SIGINT') + }) + + afterEach(() => { + process.removeAllListeners('SIGINT') + for (const listener of originalSigintListeners) { + process.on('SIGINT', listener as NodeJS.SignalsListener) + } + + sandbox.restore() + }) + + it('treats task:cancelled as a terminal event and resolves with the cancelled shape', async () => { + const stub = makeStubClient(sandbox) + const cancelled: Array<{taskId: string}> = [] + + const wait = waitForTaskCompletion( + { + client: stub.client, + command: 'curate', + format: 'text', + onCancelled: (result) => cancelled.push(result), + onCompleted() {}, + onError() {}, + taskId: 'task-1', + timeoutMs: 5000, + }, + () => {}, + ) + + // Daemon broadcasts task:cancelled + stub.emit('task:cancelled', {taskId: 'task-1'}) + + await wait + + expect(cancelled).to.have.length(1) + expect(cancelled[0]).to.include({taskId: 'task-1'}) + }) + + it('ignores task:cancelled for a different taskId', async () => { + const stub = makeStubClient(sandbox) + const cancelled: Array<{taskId: string}> = [] + + const wait = waitForTaskCompletion( + { + client: stub.client, + command: 'curate', + format: 'text', + onCancelled: (result) => cancelled.push(result), + onCompleted() {}, + onError() {}, + taskId: 'task-1', + timeoutMs: 5000, + }, + () => {}, + ) + + stub.emit('task:cancelled', {taskId: 'task-2'}) + // No terminal event for our taskId yet — emit the real one + stub.emit('task:cancelled', {taskId: 'task-1'}) + + await wait + + expect(cancelled).to.have.length(1) + expect(cancelled[0].taskId).to.equal('task-1') + }) + + it('on first SIGINT, emits task:cancel and writes a hint to stderr; does not exit', async () => { + const stub = makeStubClient(sandbox) + + const wait = waitForTaskCompletion( + { + client: stub.client, + command: 'curate', + format: 'text', + onCancelled() {}, + onCompleted() {}, + onError() {}, + taskId: 'task-1', + timeoutMs: 5000, + }, + () => {}, + ) + + process.emit('SIGINT') + + expect(stub.requestStub.calledOnce).to.equal(true) + expect(stub.requestStub.firstCall.args[0]).to.equal('task:cancel') + expect(stub.requestStub.firstCall.args[1]).to.deep.equal({taskId: 'task-1'}) + + expect(stderrWrites.some((s) => s.toLowerCase().includes('cancel'))).to.equal(true) + expect(exitStub.called).to.equal(false) + + // End the test by resolving the wait + stub.emit('task:cancelled', {taskId: 'task-1'}) + await wait + }) + + it('on second SIGINT, calls process.exit(130) without emitting another cancel', async () => { + const stub = makeStubClient(sandbox) + + const wait = waitForTaskCompletion( + { + client: stub.client, + command: 'curate', + format: 'text', + onCancelled() {}, + onCompleted() {}, + onError() {}, + taskId: 'task-1', + timeoutMs: 5000, + }, + () => {}, + ) + + process.emit('SIGINT') + expect(stub.requestStub.callCount).to.equal(1) + + process.emit('SIGINT') + expect(stub.requestStub.callCount).to.equal(1) // not re-emitted + expect(exitStub.calledOnceWithExactly(130)).to.equal(true) + + // Resolve so the test ends + stub.emit('task:cancelled', {taskId: 'task-1'}) + await wait + }) + + it('removes the SIGINT handler after wait completes via task:completed', async () => { + const stub = makeStubClient(sandbox) + const listenersBefore = process.listenerCount('SIGINT') + + const wait = waitForTaskCompletion( + { + client: stub.client, + command: 'curate', + format: 'text', + onCompleted() {}, + onError() {}, + taskId: 'task-1', + timeoutMs: 5000, + }, + () => {}, + ) + + // While waiting, exactly one SIGINT listener is installed. + expect(process.listenerCount('SIGINT')).to.equal(listenersBefore + 1) + + stub.emit('task:completed', {result: 'ok', taskId: 'task-1'}) + await wait + + expect(process.listenerCount('SIGINT')).to.equal(listenersBefore) + }) + + it('removes the SIGINT handler after wait completes via task:cancelled', async () => { + const stub = makeStubClient(sandbox) + const listenersBefore = process.listenerCount('SIGINT') + + const wait = waitForTaskCompletion( + { + client: stub.client, + command: 'curate', + format: 'text', + onCancelled() {}, + onCompleted() {}, + onError() {}, + taskId: 'task-1', + timeoutMs: 5000, + }, + () => {}, + ) + + expect(process.listenerCount('SIGINT')).to.equal(listenersBefore + 1) + + stub.emit('task:cancelled', {taskId: 'task-1'}) + await wait + + expect(process.listenerCount('SIGINT')).to.equal(listenersBefore) + }) + + it('removes the SIGINT handler after wait rejects via task:error in text mode', async () => { + const stub = makeStubClient(sandbox) + const listenersBefore = process.listenerCount('SIGINT') + + const wait = waitForTaskCompletion( + { + client: stub.client, + command: 'curate', + format: 'text', + onCompleted() {}, + onError() {}, + taskId: 'task-1', + timeoutMs: 5000, + }, + () => {}, + ) + + stub.emit('task:error', {error: {code: 'X', message: 'boom', name: 'Error'}, taskId: 'task-1'}) + + let rejected: unknown + try { + await wait + } catch (error) { + rejected = error + } + + expect(rejected).to.be.an('error') + expect(process.listenerCount('SIGINT')).to.equal(listenersBefore) + }) + + it('JSON mode hint goes to stderr, not stdout', async () => { + const stub = makeStubClient(sandbox) + const stdoutWrites: string[] = [] + sandbox.stub(process.stdout, 'write').callsFake((chunk: unknown) => { + stdoutWrites.push(String(chunk)) + return true + }) + + const wait = waitForTaskCompletion( + { + client: stub.client, + command: 'curate', + format: 'json', + onCompleted() {}, + onError() {}, + taskId: 'task-1', + timeoutMs: 5000, + }, + () => {}, + ) + + process.emit('SIGINT') + + expect(stderrStub.called).to.equal(true) + expect(stdoutWrites.some((s) => s.toLowerCase().includes('cancelling'))).to.equal(false) + + stub.emit('task:cancelled', {taskId: 'task-1'}) + await wait + }) +}) From cda4a378249c8c76bf92e91a02c11cb02ef35595 Mon Sep 17 00:00:00 2001 From: Bao Ha Date: Wed, 13 May 2026 10:24:00 +0700 Subject: [PATCH 10/21] feat: [ENG-2787] TUI cancel-task API helper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add src/tui/features/tasks/api/cancel-task.ts mirroring the provider/api/cancel-oauth pattern: pulls the active apiClient from the TUI transport store, emits task:cancel with the taskId, returns the daemon's TaskCancelResponse, throws when the response reports success: false. - No new transport client construction; reuses the established TUI pattern. Boundary preserved — no imports from server/, agent/, or oclif/. - 5 unit tests cover the event payload, the success resolve, the daemon-reported error path, the missing-error fallback, and the not-connected guard. Test file establishes the new test/unit/tui/features/tasks/api/ folder for T4.2 to extend. --- src/tui/features/tasks/api/cancel-task.ts | 25 +++++++ .../features/tasks/api/cancel-task.test.ts | 69 +++++++++++++++++++ 2 files changed, 94 insertions(+) create mode 100644 src/tui/features/tasks/api/cancel-task.ts create mode 100644 test/unit/tui/features/tasks/api/cancel-task.test.ts diff --git a/src/tui/features/tasks/api/cancel-task.ts b/src/tui/features/tasks/api/cancel-task.ts new file mode 100644 index 000000000..96ef7746d --- /dev/null +++ b/src/tui/features/tasks/api/cancel-task.ts @@ -0,0 +1,25 @@ +/** + * TUI helper: emit a task:cancel request through the active TUI transport + * client and surface the daemon's response shape to the caller. Used by the + * Ctrl+Q keybind in curate/query flow components (T4.2) and any future TUI + * surface that needs to cancel by id. + * + * Components own their cancelling-state UI; this helper only owns the + * transport emission and the result. + */ + +import { + type TaskCancelRequest, + type TaskCancelResponse, + TaskEvents, +} from '../../../../shared/transport/events/index.js' +import {useTransportStore} from '../../../stores/transport-store.js' + +export const cancelTask = async (payload: TaskCancelRequest): Promise => { + const {apiClient} = useTransportStore.getState() + if (!apiClient) throw new Error('Not connected') + + const response = await apiClient.request(TaskEvents.CANCEL, payload) + if (!response.success) throw new Error(response.error ?? 'Cancel failed') + return response +} diff --git a/test/unit/tui/features/tasks/api/cancel-task.test.ts b/test/unit/tui/features/tasks/api/cancel-task.test.ts new file mode 100644 index 000000000..9cd9a4a3e --- /dev/null +++ b/test/unit/tui/features/tasks/api/cancel-task.test.ts @@ -0,0 +1,69 @@ +import {expect} from 'chai' +import {createSandbox, type SinonSandbox, type SinonStub} from 'sinon' + +import type {BrvApiClient} from '../../../../../../src/tui/lib/api-client.js' + +import {TaskEvents} from '../../../../../../src/shared/transport/events/task-events.js' +import {cancelTask} from '../../../../../../src/tui/features/tasks/api/cancel-task.js' +import {useTransportStore} from '../../../../../../src/tui/stores/transport-store.js' + +describe('cancelTask (TUI api helper)', () => { + let sandbox: SinonSandbox + let request: SinonStub + + beforeEach(() => { + sandbox = createSandbox() + request = sandbox.stub() + useTransportStore.setState({ + apiClient: {on: sandbox.stub(), request} as unknown as BrvApiClient, + }) + }) + + afterEach(() => { + sandbox.restore() + useTransportStore.setState({apiClient: null}) + }) + + it('emits task:cancel with the taskId payload', async () => { + request.resolves({success: true}) + await cancelTask({taskId: 'tsk-1'}) + expect(request.firstCall.args[0]).to.equal(TaskEvents.CANCEL) + expect(request.firstCall.args[1]).to.deep.equal({taskId: 'tsk-1'}) + }) + + it('resolves with the daemon response on success', async () => { + request.resolves({success: true}) + const result = await cancelTask({taskId: 'tsk-1'}) + expect(result).to.deep.equal({success: true}) + }) + + it('throws with the daemon-provided error message when success: false', async () => { + request.resolves({error: 'Task not found', success: false}) + try { + await cancelTask({taskId: 'tsk-1'}) + expect.fail('expected to throw') + } catch (error) { + expect((error as Error).message).to.equal('Task not found') + } + }) + + it('falls back to "Cancel failed" when success: false has no error string', async () => { + request.resolves({success: false}) + try { + await cancelTask({taskId: 'tsk-1'}) + expect.fail('expected to throw') + } catch (error) { + expect((error as Error).message).to.equal('Cancel failed') + } + }) + + it('throws when not connected to the daemon', async () => { + useTransportStore.setState({apiClient: null}) + try { + await cancelTask({taskId: 'tsk-1'}) + expect.fail('expected to throw') + } catch (error) { + expect((error as Error).message).to.equal('Not connected') + } + }) +}) From 85151b6df9cc5340a655e72b4f2f4f56db287e2c Mon Sep 17 00:00:00 2001 From: Bao Ha Date: Wed, 13 May 2026 10:39:18 +0700 Subject: [PATCH 11/21] feat: [ENG-2788] Ctrl+Q cancel keybind for active curate/query tasks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - New useCancelRunningTaskKeybind hook installs a scoped useInput that fires only while there is a non-terminal curate/query task in the tasks store. On Ctrl+Q the hook calls cancelTask({taskId}) for the most recently created running task. - Architectural note: curate-flow and query-flow components unmount within ~100ms of task:create ack, so they cannot own a binding that must be active for the entire task lifetime. The keybind is scoped by running-task presence in the store, hosted via an invisible CancelKeybindInitializer mounted next to TaskSubscriptionInitializer. REPL UX is unchanged (prompt stays responsive while task runs). - A pure selectCancelTargetTaskId selector picks the target taskId — most recently created non-terminal task, or undefined when nothing is cancellable. Splitting the pure logic from the React hook keeps it unit-testable without Ink. - Ctrl-C remains the TUI's exit shortcut; only Ctrl+Q is intercepted. Known limitation: some terminal emulators map Ctrl+Q to XON flow- control; Ink raw mode should neutralize that on common emulators but is not guaranteed everywhere. - 6 unit tests cover the selector: empty map, all-terminal, single running, multiple running (newest wins), created-status, and terminal-newer-than-running. --- .../components/cancel-keybind-initializer.tsx | 12 ++++ .../tasks/hooks/select-cancel-target.ts | 22 +++++++ .../hooks/use-cancel-running-task-keybind.ts | 36 +++++++++++ src/tui/providers/app-providers.tsx | 2 + .../tasks/hooks/select-cancel-target.test.ts | 63 +++++++++++++++++++ 5 files changed, 135 insertions(+) create mode 100644 src/tui/features/tasks/components/cancel-keybind-initializer.tsx create mode 100644 src/tui/features/tasks/hooks/select-cancel-target.ts create mode 100644 src/tui/features/tasks/hooks/use-cancel-running-task-keybind.ts create mode 100644 test/unit/tui/features/tasks/hooks/select-cancel-target.test.ts diff --git a/src/tui/features/tasks/components/cancel-keybind-initializer.tsx b/src/tui/features/tasks/components/cancel-keybind-initializer.tsx new file mode 100644 index 000000000..abcb74c88 --- /dev/null +++ b/src/tui/features/tasks/components/cancel-keybind-initializer.tsx @@ -0,0 +1,12 @@ +/** + * Invisible component that installs the Ctrl+Q cancel keybind for curate / + * query tasks. Place inside the TransportProvider tree next to the existing + * TaskSubscriptionInitializer so it shares the same task-store source of truth. + */ + +import {useCancelRunningTaskKeybind} from '../hooks/use-cancel-running-task-keybind.js' + +export function CancelKeybindInitializer(): null { + useCancelRunningTaskKeybind() + return null +} diff --git a/src/tui/features/tasks/hooks/select-cancel-target.ts b/src/tui/features/tasks/hooks/select-cancel-target.ts new file mode 100644 index 000000000..0c1ac8523 --- /dev/null +++ b/src/tui/features/tasks/hooks/select-cancel-target.ts @@ -0,0 +1,22 @@ +import type {Task, TaskStatus} from '../stores/tasks-store.js' + +/** Terminal statuses — tasks in these states can no longer be cancelled. */ +const TERMINAL_STATUSES: ReadonlySet = new Set(['cancelled', 'completed', 'error']) + +/** + * Pick the taskId Ctrl+Q should target. Returns the most recently created + * non-terminal task in the tasks map, or undefined when there is nothing + * cancellable. Pure function — extracted from the React hook so it can be + * unit-tested without Ink. + */ +export function selectCancelTargetTaskId(tasks: ReadonlyMap): string | undefined { + let candidate: Task | undefined + for (const task of tasks.values()) { + if (TERMINAL_STATUSES.has(task.status)) continue + if (!candidate || task.createdAt > candidate.createdAt) { + candidate = task + } + } + + return candidate?.taskId +} diff --git a/src/tui/features/tasks/hooks/use-cancel-running-task-keybind.ts b/src/tui/features/tasks/hooks/use-cancel-running-task-keybind.ts new file mode 100644 index 000000000..691abdd60 --- /dev/null +++ b/src/tui/features/tasks/hooks/use-cancel-running-task-keybind.ts @@ -0,0 +1,36 @@ +/** + * Ctrl+Q keybind that cancels the most recently started curate / query task. + * + * Scoped via Ink's `useInput` `isActive` — the binding only fires while there + * is a non-terminal task in the tasks store. When no cancellable task exists, + * Ctrl+Q is a no-op so the keybind never steals input from other surfaces. + * + * Why scoped here instead of inside curate-flow / query-flow: those components + * unmount as soon as the daemon acks task:create (< 100ms), so they cannot + * own a keybind that needs to be active for the entire task lifetime. + */ + +import {useInput} from 'ink' + +import {cancelTask} from '../api/cancel-task.js' +import {useTasksStore} from '../stores/tasks-store.js' +import {selectCancelTargetTaskId} from './select-cancel-target.js' + +export function useCancelRunningTaskKeybind(): void { + const targetTaskId = useTasksStore((s) => selectCancelTargetTaskId(s.tasks)) + + useInput( + (input, key) => { + if (!targetTaskId) return + if (!key.ctrl || input !== 'q') return + + cancelTask({taskId: targetTaskId}).catch(() => { + // Best-effort: the daemon's task:cancelled (or task:error) broadcast + // is the source of truth for status. A throw here means the request + // round-trip failed, but the user's intent is captured by the keypress + // and any visible terminal event still drives the store. + }) + }, + {isActive: Boolean(targetTaskId)}, + ) +} diff --git a/src/tui/providers/app-providers.tsx b/src/tui/providers/app-providers.tsx index 7ec48cac8..04ee7767c 100644 --- a/src/tui/providers/app-providers.tsx +++ b/src/tui/providers/app-providers.tsx @@ -10,6 +10,7 @@ import React from 'react' import {AuthInitializer} from '../features/auth/components/auth-initializer.js' import {ProviderSubscriptionInitializer} from '../features/provider/components/provider-subscription-initializer.js' +import {CancelKeybindInitializer} from '../features/tasks/components/cancel-keybind-initializer.js' import {TaskSubscriptionInitializer} from '../features/tasks/components/task-subscription-initializer.js' import {TransportInitializer} from '../features/transport/components/transport-initializer.js' @@ -37,6 +38,7 @@ export function AppProviders({children}: AppProvidersProps): React.ReactElement + {children} diff --git a/test/unit/tui/features/tasks/hooks/select-cancel-target.test.ts b/test/unit/tui/features/tasks/hooks/select-cancel-target.test.ts new file mode 100644 index 000000000..886e077f6 --- /dev/null +++ b/test/unit/tui/features/tasks/hooks/select-cancel-target.test.ts @@ -0,0 +1,63 @@ +import {expect} from 'chai' + +import type {Task} from '../../../../../../src/tui/features/tasks/stores/tasks-store.js' + +import {selectCancelTargetTaskId} from '../../../../../../src/tui/features/tasks/hooks/select-cancel-target.js' + +function makeTask(overrides: Partial & Pick): Task { + return { + content: 'irrelevant', + createdAt: 0, + input: 'irrelevant', + toolCalls: [], + ...overrides, + } +} + +describe('selectCancelTargetTaskId', () => { + it('returns undefined when there are no tasks', () => { + const tasks = new Map() + expect(selectCancelTargetTaskId(tasks)).to.equal(undefined) + }) + + it('returns undefined when every task is terminal', () => { + const tasks = new Map([ + ['a', makeTask({status: 'completed', taskId: 'a', type: 'curate'})], + ['b', makeTask({status: 'cancelled', taskId: 'b', type: 'query'})], + ['c', makeTask({status: 'error', taskId: 'c', type: 'curate'})], + ]) + expect(selectCancelTargetTaskId(tasks)).to.equal(undefined) + }) + + it('returns the only running task when exactly one is non-terminal', () => { + const tasks = new Map([ + ['a', makeTask({status: 'completed', taskId: 'a', type: 'curate'})], + ['b', makeTask({createdAt: 5, status: 'started', taskId: 'b', type: 'curate'})], + ]) + expect(selectCancelTargetTaskId(tasks)).to.equal('b') + }) + + it('returns the most recently created non-terminal task when several are running', () => { + const tasks = new Map([ + ['mid', makeTask({createdAt: 200, status: 'started', taskId: 'mid', type: 'query'})], + ['new', makeTask({createdAt: 300, status: 'started', taskId: 'new', type: 'curate'})], + ['old', makeTask({createdAt: 100, status: 'started', taskId: 'old', type: 'curate'})], + ]) + expect(selectCancelTargetTaskId(tasks)).to.equal('new') + }) + + it('treats `created` status as non-terminal (still cancellable before task:started)', () => { + const tasks = new Map([ + ['a', makeTask({createdAt: 10, status: 'created', taskId: 'a', type: 'curate'})], + ]) + expect(selectCancelTargetTaskId(tasks)).to.equal('a') + }) + + it('ignores terminal siblings even when newer than running ones', () => { + const tasks = new Map([ + ['fresh-but-done', makeTask({createdAt: 200, status: 'completed', taskId: 'fresh-but-done', type: 'curate'})], + ['running', makeTask({createdAt: 100, status: 'started', taskId: 'running', type: 'curate'})], + ]) + expect(selectCancelTargetTaskId(tasks)).to.equal('running') + }) +}) From c19bf938d46f7467a6dd6132b23f30daf595d011 Mon Sep 17 00:00:00 2001 From: Bao Ha Date: Wed, 13 May 2026 11:39:04 +0700 Subject: [PATCH 12/21] refactor: [ENG-2783] DRY cancel-branch wiring + tighten SIGINT and query typing Post-review cleanup of T2.5 (ENG-2783): - Extract runCancelBranchWithRetry into src/oclif/lib/cancel-task.ts so curate / query / dream stop cloning the same 22-line withDaemonRetry wrapper. Per-command runCancelBranch becomes a 10-line delegation: pass command name, daemon-client options, format, log, transport-error handler, and taskId; receive a boolean for exit-code decision. - Wrap the SIGINT handler's client.request call in try/catch so a synchronously-throwing transport (already-disconnected socket, etc.) cannot escape the signal handler and crash the process with an uncaught exception. The wait loop still surfaces the disconnect via timeout or state-change paths. - Drop the redundant 'as QueryFlags' cast in query.ts entirely along with the now-unused QueryFlags type alias. Use oclif's inferred flag type directly and narrow 'format' inline via a ternary, matching the pattern in curate/index.ts. - 3 new unit tests for runCancelBranchWithRetry: success path, daemon- reported failure (no transport throw), and transport-throw routed to onTransportError. Existing per-command --cancel tests pass unchanged because the per-command behaviour is the same. --- src/oclif/commands/curate/index.ts | 30 ++----- src/oclif/commands/dream.ts | 30 ++----- src/oclif/commands/query.ts | 41 +++------ src/oclif/lib/cancel-task.ts | 38 ++++++++ src/oclif/lib/task-client.ts | 11 ++- .../lib/run-cancel-branch-with-retry.test.ts | 88 +++++++++++++++++++ 6 files changed, 164 insertions(+), 74 deletions(-) create mode 100644 test/unit/oclif/lib/run-cancel-branch-with-retry.test.ts diff --git a/src/oclif/commands/curate/index.ts b/src/oclif/commands/curate/index.ts index a5097ffe4..becb9b93b 100644 --- a/src/oclif/commands/curate/index.ts +++ b/src/oclif/commands/curate/index.ts @@ -9,7 +9,7 @@ import {BRV_DIR, CONTEXT_TREE_DIR} from '../../../server/constants.js' import {ProviderConfigResponse, TransportStateEventNames} from '../../../server/core/domain/transport/index.js' import {extractCurateOperations} from '../../../server/utils/curate-result-parser.js' import {TaskEvents} from '../../../shared/transport/events/index.js' -import {runCancelTask} from '../../lib/cancel-task.js' +import {runCancelBranchWithRetry} from '../../lib/cancel-task.js' import { type DaemonClientOptions, formatConnectionError, @@ -297,26 +297,14 @@ Bad examples: } private async runCancelBranch(taskId: string, format: 'json' | 'text'): Promise { - let success = false - try { - await withDaemonRetry( - async (client) => { - success = await runCancelTask({ - client, - command: 'curate', - format, - log: (msg) => this.log(msg), - taskId, - }) - }, - this.getDaemonClientOptions(), - ) - } catch (error) { - this.reportError(error, format) - this.exit(1) - return - } - + const success = await runCancelBranchWithRetry({ + command: 'curate', + daemonClientOptions: this.getDaemonClientOptions(), + format, + log: (msg) => this.log(msg), + onTransportError: (error) => this.reportError(error, format), + taskId, + }) if (!success) this.exit(1) } diff --git a/src/oclif/commands/dream.ts b/src/oclif/commands/dream.ts index 1f6a27b4c..0d3fb6741 100644 --- a/src/oclif/commands/dream.ts +++ b/src/oclif/commands/dream.ts @@ -22,7 +22,7 @@ import {FileCurateLogStore} from '../../server/infra/storage/file-curate-log-sto import {FileReviewBackupStore} from '../../server/infra/storage/file-review-backup-store.js' import {getProjectDataDir} from '../../server/utils/path-utils.js' import {TaskEvents} from '../../shared/transport/events/index.js' -import {runCancelTask} from '../lib/cancel-task.js' +import {runCancelBranchWithRetry} from '../lib/cancel-task.js' import { type DaemonClientOptions, formatConnectionError, @@ -192,26 +192,14 @@ export default class Dream extends Command { } private async runCancelBranch(taskId: string, format: 'json' | 'text'): Promise { - let success = false - try { - await withDaemonRetry( - async (client) => { - success = await runCancelTask({ - client, - command: 'dream', - format, - log: (msg) => this.log(msg), - taskId, - }) - }, - this.getDaemonClientOptions(), - ) - } catch (error) { - this.reportError(error, format) - this.exit(1) - return - } - + const success = await runCancelBranchWithRetry({ + command: 'dream', + daemonClientOptions: this.getDaemonClientOptions(), + format, + log: (msg) => this.log(msg), + onTransportError: (error) => this.reportError(error, format), + taskId, + }) if (!success) this.exit(1) } diff --git a/src/oclif/commands/query.ts b/src/oclif/commands/query.ts index 3fdd93cae..3038c5941 100644 --- a/src/oclif/commands/query.ts +++ b/src/oclif/commands/query.ts @@ -5,7 +5,7 @@ import {randomUUID} from 'node:crypto' import {type ProviderConfigResponse, TransportStateEventNames} from '../../server/core/domain/transport/schemas.js' import {TaskEvents} from '../../shared/transport/events/index.js' -import {runCancelTask} from '../lib/cancel-task.js' +import {runCancelBranchWithRetry} from '../lib/cancel-task.js' import { type DaemonClientOptions, formatConnectionError, @@ -17,12 +17,6 @@ import { import {writeJsonResponse} from '../lib/json-response.js' import {DEFAULT_TIMEOUT_SECONDS, MAX_TIMEOUT_SECONDS, MIN_TIMEOUT_SECONDS, waitForTaskCompletion} from '../lib/task-client.js' -/** Parsed flags type */ -type QueryFlags = { - format?: 'json' | 'text' - timeout?: number -} - export default class Query extends Command { public static args = { query: Args.string({ @@ -69,9 +63,8 @@ Bad: } public async run(): Promise { - const {args, flags: rawFlags} = await this.parse(Query) - const flags = rawFlags as QueryFlags & {cancel?: string} - const format = (flags.format ?? 'text') as 'json' | 'text' + const {args, flags} = await this.parse(Query) + const format: 'json' | 'text' = flags.format === 'json' ? 'json' : 'text' if (flags.cancel) { if (args.query !== undefined && args.query.trim() !== '') { @@ -158,26 +151,14 @@ Bad: } private async runCancelBranch(taskId: string, format: 'json' | 'text'): Promise { - let success = false - try { - await withDaemonRetry( - async (client) => { - success = await runCancelTask({ - client, - command: 'query', - format, - log: (msg) => this.log(msg), - taskId, - }) - }, - this.getDaemonClientOptions(), - ) - } catch (error) { - this.reportError(error, format) - this.exit(1) - return - } - + const success = await runCancelBranchWithRetry({ + command: 'query', + daemonClientOptions: this.getDaemonClientOptions(), + format, + log: (msg) => this.log(msg), + onTransportError: (error) => this.reportError(error, format), + taskId, + }) if (!success) this.exit(1) } diff --git a/src/oclif/lib/cancel-task.ts b/src/oclif/lib/cancel-task.ts index 8561533a1..4db95dfb4 100644 --- a/src/oclif/lib/cancel-task.ts +++ b/src/oclif/lib/cancel-task.ts @@ -13,6 +13,7 @@ import type {ITransportClient} from '@campfirein/brv-transport-client' import type {TaskCancelRequest, TaskCancelResponse} from '../../shared/transport/events/task-events.js' import {TaskEvents} from '../../shared/transport/events/task-events.js' +import {type DaemonClientOptions, withDaemonRetry} from './daemon-client.js' import {writeJsonResponse} from './json-response.js' export type RunCancelTaskOptions = { @@ -60,3 +61,40 @@ export async function runCancelTask(options: RunCancelTaskOptions): Promise this.log(msg)` in oclif). */ + log: (msg: string) => void + /** Called when `withDaemonRetry` rethrows. Caller decides what to print and whether to exit. */ + onTransportError: (error: unknown) => void + /** Task to cancel. */ + taskId: string +} + +/** + * Reusable cancel-branch wiring for oclif commands' `--cancel ` flag. + * Runs `runCancelTask` inside `withDaemonRetry`, surfaces transport errors + * via the supplied callback, and returns the helper's success flag so the + * caller can decide on exit code. Single point of evolution if the cancel + * pipeline ever grows extra retry/finalization concerns. + */ +export async function runCancelBranchWithRetry(options: RunCancelBranchOptions): Promise { + const {command, daemonClientOptions, format, log, onTransportError, taskId} = options + let success = false + try { + await withDaemonRetry(async (client) => { + success = await runCancelTask({client, command, format, log, taskId}) + }, daemonClientOptions) + } catch (error) { + onTransportError(error) + return false + } + + return success +} diff --git a/src/oclif/lib/task-client.ts b/src/oclif/lib/task-client.ts index f137540a0..c6ec62b13 100644 --- a/src/oclif/lib/task-client.ts +++ b/src/oclif/lib/task-client.ts @@ -188,8 +188,15 @@ function installSigintCancel(deps: {client: ITransportClient; taskId: string}): // Fire-and-forget; we await the daemon's task:cancelled broadcast in the // wait loop. The daemon may also respond with success/false on a stale id; // we ignore that here because the wait loop times out or sees a terminal - // event either way. - deps.client.request(TaskEvents.CANCEL, {taskId: deps.taskId}) + // event either way. Wrapped in try/catch so a synchronously-throwing + // transport (already-disconnected socket, etc.) doesn't propagate out of + // the signal handler and crash the process with an uncaught exception. + try { + deps.client.request(TaskEvents.CANCEL, {taskId: deps.taskId}) + } catch { + // Transport unavailable — the wait loop will time out or see a + // disconnect event and surface that path instead. + } } process.on('SIGINT', onSigint) diff --git a/test/unit/oclif/lib/run-cancel-branch-with-retry.test.ts b/test/unit/oclif/lib/run-cancel-branch-with-retry.test.ts new file mode 100644 index 000000000..107ee5914 --- /dev/null +++ b/test/unit/oclif/lib/run-cancel-branch-with-retry.test.ts @@ -0,0 +1,88 @@ +import type {ITransportClient} from '@campfirein/brv-transport-client' + +import {expect} from 'chai' +import {createSandbox, type SinonSandbox, type SinonStub} from 'sinon' + +import {runCancelBranchWithRetry} from '../../../../src/oclif/lib/cancel-task.js' + +describe('runCancelBranchWithRetry', () => { + let sandbox: SinonSandbox + let requestStub: SinonStub + let logCalls: string[] + let transportErrors: unknown[] + let stdoutWriteStub: SinonStub + + beforeEach(() => { + sandbox = createSandbox() + requestStub = sandbox.stub() + logCalls = [] + transportErrors = [] + stdoutWriteStub = sandbox.stub(process.stdout, 'write').returns(true) + }) + + afterEach(() => { + sandbox.restore() + }) + + /** + * Inject a fake `transportConnector` so the helper goes through real + * `withDaemonRetry` plumbing without needing a live daemon. The connector + * always returns the same in-memory client; the retry behaviour itself is + * covered by `daemon-client` unit tests — what we verify here is the + * cancel-branch wiring around it. + */ + function makeOptions() { + const fakeClient = { + disconnect: sandbox.stub().resolves(), + requestWithAck: requestStub, + } as unknown as ITransportClient + + return { + command: 'curate', + daemonClientOptions: { + maxRetries: 1, + retryDelayMs: 0, + transportConnector: async () => ({ + client: fakeClient, + projectRoot: '/proj', + }), + }, + format: 'text' as const, + log: (msg: string) => logCalls.push(msg), + onTransportError: (error: unknown) => transportErrors.push(error), + taskId: 'task-1', + } + } + + it('returns true and prints the success line when the daemon reports success', async () => { + requestStub.resolves({success: true}) + + const result = await runCancelBranchWithRetry(makeOptions()) + + expect(result).to.equal(true) + expect(logCalls).to.include('Cancelled task-1') + expect(transportErrors).to.have.length(0) + expect(stdoutWriteStub.called).to.equal(false) + }) + + it('returns false when the daemon reports failure (no transport throw)', async () => { + requestStub.resolves({error: 'Task not found', success: false}) + + const result = await runCancelBranchWithRetry(makeOptions()) + + expect(result).to.equal(false) + expect(logCalls.some((l) => l.includes('Failed to cancel task-1') && l.includes('Task not found'))).to.equal(true) + expect(transportErrors).to.have.length(0) + }) + + it('returns false and invokes onTransportError when withDaemonRetry rethrows', async () => { + const boom = new Error('connection refused') + requestStub.rejects(boom) + + const result = await runCancelBranchWithRetry(makeOptions()) + + expect(result).to.equal(false) + expect(transportErrors).to.have.length(1) + expect(transportErrors[0]).to.equal(boom) + }) +}) From 39720dc34bf450439641e1a78b899319f0017d24 Mon Sep 17 00:00:00 2001 From: Bao Ha Date: Wed, 13 May 2026 11:50:32 +0700 Subject: [PATCH 13/21] feat: [ENG-2783] foreground commands surface remote --cancel MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the foreground curate / query / dream wait loop sees a task:cancelled broadcast (driven by a remote 'brv --cancel ' from another terminal, or the TUI Ctrl+Q keybind), the command now: - prints '✗ cancelled (Task: )' in text mode, or writes a {status: 'cancelled', event: 'cancelled'} JSON envelope in json mode, so consumers can distinguish cancel from success; - exits with code 130 (the conventional SIGINT exit) so shell scripts and CI can branch on cancellation. Wiring detail worth noting: the wasCancelled flag is hoisted to run() and bubbled up by submitTask via its return shape. The this.exit(130) call lives AFTER the withDaemonRetry try/catch — throwing it inside the callback would be swallowed by the catch (which routes any throw through reportError) and the exit code would be lost. 6 new unit tests cover the remote-cancel path in text and json mode across all three commands. Existing tests still pass unchanged. --- src/oclif/commands/curate/index.ts | 28 +++++++++++++++-- src/oclif/commands/dream.ts | 28 +++++++++++++++-- src/oclif/commands/query.ts | 26 ++++++++++++++-- test/commands/curate.test.ts | 50 ++++++++++++++++++++++++++++++ test/commands/dream.test.ts | 50 ++++++++++++++++++++++++++++++ test/commands/query.test.ts | 50 ++++++++++++++++++++++++++++++ 6 files changed, 226 insertions(+), 6 deletions(-) diff --git a/src/oclif/commands/curate/index.ts b/src/oclif/commands/curate/index.ts index becb9b93b..d7e30c7bb 100644 --- a/src/oclif/commands/curate/index.ts +++ b/src/oclif/commands/curate/index.ts @@ -130,6 +130,7 @@ Bad examples: const taskType = flags.folder?.length ? 'curate-folder' : 'curate' let providerContext: ProviderErrorContext | undefined + let wasCancelled = false try { await withDaemonRetry( @@ -149,7 +150,8 @@ Bad examples: throw new Error(providerMissingMessage(active.activeProvider, active.authMethod)) } - await this.submitTask({client, content: resolvedContent, flags, format, projectRoot, taskType, worktreeRoot}) + const result = await this.submitTask({client, content: resolvedContent, flags, format, projectRoot, taskType, worktreeRoot}) + if (result.wasCancelled) wasCancelled = true }, { ...this.getDaemonClientOptions(), @@ -162,7 +164,13 @@ Bad examples: ) } catch (error) { this.reportError(error, format, providerContext) + return } + + // Throw the SIGINT-conventional exit AFTER the daemon-retry try/catch so + // the ExitError isn't swallowed by reportError. Routine completions and + // errors fall through here naturally. + if (wasCancelled) this.exit(130) } /** @@ -316,7 +324,7 @@ Bad examples: projectRoot?: string taskType: string worktreeRoot?: string - }): Promise { + }): Promise<{wasCancelled: boolean}> { const {client, content, flags, format, projectRoot, taskType, worktreeRoot} = props const hasFolders = Boolean(flags.folder?.length) const taskId = randomUUID() @@ -350,11 +358,24 @@ Bad examples: this.log(`✓ Context queued for processing.${suffix}`) } } else { + let wasCancelled = false const completionPromise = waitForTaskCompletion( { client, command: 'curate', format, + onCancelled: ({taskId: tid}) => { + wasCancelled = true + if (format === 'json') { + writeJsonResponse({ + command: 'curate', + data: {event: 'cancelled', message: 'Curate cancelled', status: 'cancelled', taskId: tid}, + success: true, + }) + } else { + this.log(`✗ Curate cancelled (Task: ${tid})`) + } + }, onCompleted: ({logId, pendingReview, taskId: tid, toolCalls}) => { const changes = this.composeChangesFromToolCalls(toolCalls) // Per-file detail is best-effort enrichment; server notify is authoritative @@ -409,7 +430,10 @@ Bad examples: ) await client.requestWithAck(TaskEvents.CREATE, taskPayload) await completionPromise + return {wasCancelled} } + + return {wasCancelled: false} } private validateInput(args: {context?: string}, flags: CurateFlags, format: 'json' | 'text'): boolean { diff --git a/src/oclif/commands/dream.ts b/src/oclif/commands/dream.ts index 0d3fb6741..0327b370c 100644 --- a/src/oclif/commands/dream.ts +++ b/src/oclif/commands/dream.ts @@ -133,6 +133,7 @@ export default class Dream extends Command { } let providerContext: ProviderErrorContext | undefined + let wasCancelled = false try { await withDaemonRetry( @@ -152,7 +153,7 @@ export default class Dream extends Command { throw new Error(providerMissingMessage(active.activeProvider, active.authMethod)) } - await this.submitTask({ + const result = await this.submitTask({ client, detach: rawFlags.detach, force: rawFlags.force, @@ -161,6 +162,7 @@ export default class Dream extends Command { timeout: rawFlags.timeout ?? DEFAULT_TIMEOUT_SECONDS, worktreeRoot, }) + if (result.wasCancelled) wasCancelled = true }, { ...this.getDaemonClientOptions(), @@ -173,7 +175,13 @@ export default class Dream extends Command { ) } catch (error) { this.reportError(error, format, providerContext) + return } + + // Throw the SIGINT-conventional exit AFTER the daemon-retry try/catch so + // the ExitError isn't swallowed by reportError. Routine completions and + // errors fall through here naturally. + if (wasCancelled) this.exit(130) } private reportError(error: unknown, format: 'json' | 'text', providerContext?: ProviderErrorContext): void { @@ -245,7 +253,7 @@ export default class Dream extends Command { projectRoot?: string timeout: number worktreeRoot?: string - }): Promise { + }): Promise<{wasCancelled: boolean}> { const {client, detach, force, format, projectRoot, timeout, worktreeRoot} = props const taskId = randomUUID() const taskPayload = { @@ -276,11 +284,24 @@ export default class Dream extends Command { this.log(`✓ Dream queued for processing.${logSuffix}`) } } else { + let wasCancelled = false const completionPromise = waitForTaskCompletion( { client, command: 'dream', format, + onCancelled: ({taskId: tid}) => { + wasCancelled = true + if (format === 'json') { + writeJsonResponse({ + command: 'dream', + data: {event: 'cancelled', message: 'Dream cancelled', status: 'cancelled', taskId: tid}, + success: true, + }) + } else { + this.log(`✗ Dream cancelled (Task: ${tid})`) + } + }, onCompleted: ({logId, result, taskId: tid}) => { const skipped = result?.startsWith('Dream skipped:') if (format === 'json') { @@ -313,6 +334,9 @@ export default class Dream extends Command { ) await client.requestWithAck(TaskEvents.CREATE, taskPayload) await completionPromise + return {wasCancelled} } + + return {wasCancelled: false} } } diff --git a/src/oclif/commands/query.ts b/src/oclif/commands/query.ts index 3038c5941..a3cdbe9c6 100644 --- a/src/oclif/commands/query.ts +++ b/src/oclif/commands/query.ts @@ -80,6 +80,7 @@ Bad: if (!this.validateInput(args.query ?? '', format)) return let providerContext: ProviderErrorContext | undefined + let wasCancelled = false try { await withDaemonRetry( @@ -99,7 +100,7 @@ Bad: throw new Error(providerMissingMessage(active.activeProvider, active.authMethod)) } - await this.submitTask({ + const result = await this.submitTask({ client, format, projectRoot, @@ -107,6 +108,7 @@ Bad: timeoutMs: (flags.timeout ?? DEFAULT_TIMEOUT_SECONDS) * 1000, worktreeRoot, }) + if (result.wasCancelled) wasCancelled = true }, { ...this.getDaemonClientOptions(), @@ -119,7 +121,13 @@ Bad: ) } catch (error) { this.reportError(error, format, providerContext) + return } + + // Throw the SIGINT-conventional exit AFTER the daemon-retry try/catch so + // the ExitError isn't swallowed by reportError. Routine completions and + // errors fall through here naturally. + if (wasCancelled) this.exit(130) } private reportCombinationError(format: 'json' | 'text'): void { @@ -169,7 +177,7 @@ Bad: query: string timeoutMs?: number worktreeRoot?: string - }): Promise { + }): Promise<{wasCancelled: boolean}> { const {client, format, projectRoot, query, timeoutMs, worktreeRoot} = props const taskId = randomUUID() const taskPayload = { @@ -182,12 +190,25 @@ Bad: } let finalResult: string | undefined + let wasCancelled = false const completionPromise = waitForTaskCompletion( { client, command: 'query', format, + onCancelled: ({taskId: tid}) => { + wasCancelled = true + if (format === 'json') { + writeJsonResponse({ + command: 'query', + data: {event: 'cancelled', message: 'Query cancelled', status: 'cancelled', taskId: tid}, + success: true, + }) + } else { + this.log(`✗ Query cancelled (Task: ${tid})`) + } + }, onCompleted: ({durationMs, matchedDocs, result, taskId: tid, tier, topScore}) => { const previousResult = finalResult @@ -259,6 +280,7 @@ Bad: ) await client.requestWithAck(TaskEvents.CREATE, taskPayload) await completionPromise + return {wasCancelled} } private validateInput(query: string, format: 'json' | 'text'): boolean { diff --git a/test/commands/curate.test.ts b/test/commands/curate.test.ts index f967707b3..2e93ca1ad 100644 --- a/test/commands/curate.test.ts +++ b/test/commands/curate.test.ts @@ -710,4 +710,54 @@ describe('Curate Command', () => { expect(eventNames).to.deep.equal(['task:cancel']) }) }) + + // ==================== Remote cancel during foreground wait (N-2) ==================== + + describe('remote cancel during foreground wait', () => { + // eslint-disable-next-line unicorn/consistent-function-scoping -- captures mockClient from outer beforeEach + function simulateRemoteCancel(): void { + const eventHandlers = new Map void>() + ;(mockClient.on as sinon.SinonStub).callsFake((event: string, handler: (data: unknown) => void) => { + eventHandlers.set(event, handler) + return () => {} + }) + ;(mockClient.requestWithAck as sinon.SinonStub).callsFake(async (event: string, data: unknown) => { + if (event === 'state:getProviderConfig') return {activeProvider: 'anthropic'} + const {taskId} = data as {taskId: string} + setImmediate(() => { + eventHandlers.get('task:cancelled')?.({taskId}) + }) + return {logId: 'log-1'} + }) + } + + it('prints the cancelled line and exits non-zero (text)', async () => { + simulateRemoteCancel() + + let exitError: unknown + try { + await createCommand('test context').run() + } catch (error) { + exitError = error + } + + expect(loggedMessages.some((m) => m.includes('Curate cancelled'))).to.be.true + expect(exitError).to.not.equal(undefined) + }) + + it('emits a cancelled JSON envelope and exits non-zero (json)', async () => { + simulateRemoteCancel() + + try { + await createJsonCommand('test context').run() + } catch { + // ExitError on this.exit(130) + } + + const json = parseLastJsonLine() + expect(json.command).to.equal('curate') + expect(json.success).to.equal(true) + expect(json.data).to.deep.include({event: 'cancelled', status: 'cancelled'}) + }) + }) }) diff --git a/test/commands/dream.test.ts b/test/commands/dream.test.ts index 3dcfd6cad..d2fbc42d0 100644 --- a/test/commands/dream.test.ts +++ b/test/commands/dream.test.ts @@ -290,4 +290,54 @@ describe('Dream Command', () => { expect(eventNames).to.deep.equal(['task:cancel']) }) }) + + // ==================== Remote cancel during foreground wait (N-2) ==================== + + describe('remote cancel during foreground wait', () => { + // eslint-disable-next-line unicorn/consistent-function-scoping -- captures mockClient from outer beforeEach + function simulateRemoteCancel(): void { + const eventHandlers = new Map void>() + ;(mockClient.on as sinon.SinonStub).callsFake((event: string, handler: (data: unknown) => void) => { + eventHandlers.set(event, handler) + return () => {} + }) + ;(mockClient.requestWithAck as sinon.SinonStub).callsFake(async (event: string, data: unknown) => { + if (event === 'state:getProviderConfig') return {activeProvider: 'anthropic'} + const {taskId} = data as {taskId: string} + setImmediate(() => { + eventHandlers.get('task:cancelled')?.({taskId}) + }) + return {logId: 'log-1'} + }) + } + + it('prints the cancelled line and exits non-zero (text)', async () => { + simulateRemoteCancel() + + let exitError: unknown + try { + await createCommand().run() + } catch (error) { + exitError = error + } + + expect(loggedMessages.some((m) => m.includes('Dream cancelled'))).to.be.true + expect(exitError).to.not.equal(undefined) + }) + + it('emits a cancelled JSON envelope and exits non-zero (json)', async () => { + simulateRemoteCancel() + + try { + await createJsonCommand().run() + } catch { + // ExitError on this.exit(130) + } + + const json = parseJsonOutput() + expect(json.command).to.equal('dream') + expect(json.success).to.equal(true) + expect(json.data).to.deep.include({event: 'cancelled', status: 'cancelled'}) + }) + }) }) diff --git a/test/commands/query.test.ts b/test/commands/query.test.ts index c9ff029c6..ac5dee6ed 100644 --- a/test/commands/query.test.ts +++ b/test/commands/query.test.ts @@ -739,4 +739,54 @@ describe('Query Command', () => { expect(loggedMissing || exitError !== undefined).to.equal(true) }) }) + + // ==================== Remote cancel during foreground wait (N-2) ==================== + + describe('remote cancel during foreground wait', () => { + // eslint-disable-next-line unicorn/consistent-function-scoping -- captures mockClient from outer beforeEach + function simulateRemoteCancel(): void { + const eventHandlers = new Map void>() + ;(mockClient.on as sinon.SinonStub).callsFake((event: string, handler: (data: unknown) => void) => { + eventHandlers.set(event, handler) + return () => {} + }) + ;(mockClient.requestWithAck as sinon.SinonStub).callsFake(async (event: string, data: unknown) => { + if (event === 'state:getProviderConfig') return {activeProvider: 'anthropic'} + const {taskId} = data as {taskId: string} + setImmediate(() => { + eventHandlers.get('task:cancelled')?.({taskId}) + }) + return {taskId} + }) + } + + it('prints the cancelled line and exits non-zero (text)', async () => { + simulateRemoteCancel() + + let exitError: unknown + try { + await createCommand('test query').run() + } catch (error) { + exitError = error + } + + expect(loggedMessages.some((m) => m.includes('Query cancelled'))).to.be.true + expect(exitError).to.not.equal(undefined) + }) + + it('emits a cancelled JSON envelope and exits non-zero (json)', async () => { + simulateRemoteCancel() + + try { + await createJsonCommand('test query').run() + } catch { + // ExitError on this.exit(130) + } + + const lines = parseJsonOutput() + const cancelled = lines.find((line) => line.data.status === 'cancelled') + expect(cancelled).to.not.equal(undefined) + expect(cancelled!.command).to.equal('query') + }) + }) }) From d57edbd770c3014be85ed6571a14d214e3b56cb8 Mon Sep 17 00:00:00 2001 From: Bao Ha Date: Wed, 13 May 2026 15:13:10 +0700 Subject: [PATCH 14/21] fix: [ENG-2783] daemon-side idempotency for task:cancel retries When withDaemonRetry retries a task:cancel after a dropped response, the daemon's handleTaskCancel would look up the taskId in this.tasks, miss (the prior cancel had already moved it to completedTasks), and return {error: 'Task not found', success: false}. The CLI then reported a misleading 'Failed to cancel' even though the cancel actually worked. Make the lookup idempotent: if the taskId is absent from this.tasks but present in completedTasks with status: 'cancelled', return success. Truly unknown taskIds and tasks that reached a different terminal state (completed / error) still return the structured error, so the existing contract for those cases is preserved. This covers Nit-3 from the cancel-pipeline post-review. 1 new unit test in test/unit/infra/process/task-router.test.ts asserts the retry-after-cancel path returns {success: true}. Existing tests including 'should return error for unknown taskId' continue to pass. --- src/server/infra/process/task-router.ts | 10 ++++++++++ test/unit/infra/process/task-router.test.ts | 15 +++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/src/server/infra/process/task-router.ts b/src/server/infra/process/task-router.ts index 94f4a13d9..1f52f4a83 100644 --- a/src/server/infra/process/task-router.ts +++ b/src/server/infra/process/task-router.ts @@ -639,6 +639,16 @@ export class TaskRouter { const task = this.tasks.get(taskId) if (!task) { + // Idempotency: if the task already reached status: 'cancelled' on a + // prior cancel, return success so a retry (e.g. withDaemonRetry on a + // dropped response) isn't reported as a misleading "Task not found" + // failure. Any other terminal state (completed / error) — or a truly + // unknown taskId — still returns the structured error. + const prior = this.completedTasks.get(taskId) + if (prior?.task.status === 'cancelled') { + return {success: true} + } + return {error: 'Task not found', success: false} } diff --git a/test/unit/infra/process/task-router.test.ts b/test/unit/infra/process/task-router.test.ts index 45c8e6d13..468e38c68 100644 --- a/test/unit/infra/process/task-router.test.ts +++ b/test/unit/infra/process/task-router.test.ts @@ -851,6 +851,21 @@ describe('TaskRouter', () => { expect(result).to.deep.equal({error: 'Task not found', success: false}) }) + it('returns success on a retry after the task was already cancelled (idempotent)', () => { + // First cancel succeeds and moves the task into completedTasks via + // handleTaskCancelled. A retry (e.g. due to a dropped response in + // withDaemonRetry) sees the task gone from this.tasks but still + // present in completedTasks with status: 'cancelled' — the daemon + // returns success so the retry isn't reported as a misleading failure. + const cancelledHandler = transportHelper.requestHandlers.get(TransportTaskEventNames.CANCELLED) + cancelledHandler!({taskId}, 'agent-1') + + const cancelHandler = transportHelper.requestHandlers.get(TransportTaskEventNames.CANCEL) + const result = cancelHandler!({taskId}, 'client-1') + + expect(result).to.deep.equal({success: true}) + }) + describe('queued task cancellation (T1.3)', () => { it('cancels a queued task directly via agentPool, broadcasts cancelled, never forwards to agent', () => { agentPool.cancelQueuedTask.returns(true) From c6d77da0b351994182fa2383374dab97136946ea Mon Sep 17 00:00:00 2001 From: Bao Ha Date: Wed, 13 May 2026 15:21:25 +0700 Subject: [PATCH 15/21] chore: [ENG-2783] log swallowed SIGINT errors + align cancel JSON success flag MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two small post-review nits from the cancel-pipeline review: Nit-4 — empty catch {} in the SIGINT handler hid programmer-error throws (bad payload, transport regression) silently. Add a single stderr write inside the catch so a genuine bug leaves a breadcrumb. Comment expanded to spell out why we still swallow rather than propagate (the wait loop's timeout / state-change paths already surface a true transport drop). Nit-5 — the cancelled-path JSON envelope previously set success: true while the process exits 130. Two signals pointing opposite ways confused consumers parsing both. Flip JSON success to false so it tracks the exit code; cancellation semantics still live in data.status: 'cancelled' for code that needs to distinguish cancel from error. Updated the 3 'remote cancel during foreground wait' JSON tests across curate/query/dream to expect success: false. No behaviour change in text mode or in exit codes. Touches src/oclif/lib/task-client.ts and the 3 CLI command files only; test/commands/{curate,query,dream}.test.ts get the matching assertion flip. --- src/oclif/commands/curate/index.ts | 4 +++- src/oclif/commands/dream.ts | 4 +++- src/oclif/commands/query.ts | 4 +++- src/oclif/lib/task-client.ts | 8 ++++++-- test/commands/curate.test.ts | 3 ++- test/commands/dream.test.ts | 3 ++- test/commands/query.test.ts | 2 ++ 7 files changed, 21 insertions(+), 7 deletions(-) diff --git a/src/oclif/commands/curate/index.ts b/src/oclif/commands/curate/index.ts index d7e30c7bb..948d05e57 100644 --- a/src/oclif/commands/curate/index.ts +++ b/src/oclif/commands/curate/index.ts @@ -367,10 +367,12 @@ Bad examples: onCancelled: ({taskId: tid}) => { wasCancelled = true if (format === 'json') { + // success: false because the JSON top-level field tracks the exit + // code (130 on cancel). Cancellation semantics live in data.status. writeJsonResponse({ command: 'curate', data: {event: 'cancelled', message: 'Curate cancelled', status: 'cancelled', taskId: tid}, - success: true, + success: false, }) } else { this.log(`✗ Curate cancelled (Task: ${tid})`) diff --git a/src/oclif/commands/dream.ts b/src/oclif/commands/dream.ts index 0327b370c..bbd6706dc 100644 --- a/src/oclif/commands/dream.ts +++ b/src/oclif/commands/dream.ts @@ -293,10 +293,12 @@ export default class Dream extends Command { onCancelled: ({taskId: tid}) => { wasCancelled = true if (format === 'json') { + // success: false because the JSON top-level field tracks the exit + // code (130 on cancel). Cancellation semantics live in data.status. writeJsonResponse({ command: 'dream', data: {event: 'cancelled', message: 'Dream cancelled', status: 'cancelled', taskId: tid}, - success: true, + success: false, }) } else { this.log(`✗ Dream cancelled (Task: ${tid})`) diff --git a/src/oclif/commands/query.ts b/src/oclif/commands/query.ts index a3cdbe9c6..b9d22049e 100644 --- a/src/oclif/commands/query.ts +++ b/src/oclif/commands/query.ts @@ -200,10 +200,12 @@ Bad: onCancelled: ({taskId: tid}) => { wasCancelled = true if (format === 'json') { + // success: false because the JSON top-level field tracks the exit + // code (130 on cancel). Cancellation semantics live in data.status. writeJsonResponse({ command: 'query', data: {event: 'cancelled', message: 'Query cancelled', status: 'cancelled', taskId: tid}, - success: true, + success: false, }) } else { this.log(`✗ Query cancelled (Task: ${tid})`) diff --git a/src/oclif/lib/task-client.ts b/src/oclif/lib/task-client.ts index c6ec62b13..8f5c683e5 100644 --- a/src/oclif/lib/task-client.ts +++ b/src/oclif/lib/task-client.ts @@ -193,9 +193,13 @@ function installSigintCancel(deps: {client: ITransportClient; taskId: string}): // the signal handler and crash the process with an uncaught exception. try { deps.client.request(TaskEvents.CANCEL, {taskId: deps.taskId}) - } catch { + } catch (error) { // Transport unavailable — the wait loop will time out or see a - // disconnect event and surface that path instead. + // disconnect event and surface that path instead. Log the swallowed + // throw to stderr so a genuine bug (bad payload, transport regression) + // leaves a breadcrumb instead of failing silently. + const message = error instanceof Error ? error.message : String(error) + process.stderr.write(`[brv] cancel request failed locally: ${message}\n`) } } diff --git a/test/commands/curate.test.ts b/test/commands/curate.test.ts index 2e93ca1ad..aadca215e 100644 --- a/test/commands/curate.test.ts +++ b/test/commands/curate.test.ts @@ -756,7 +756,8 @@ describe('Curate Command', () => { const json = parseLastJsonLine() expect(json.command).to.equal('curate') - expect(json.success).to.equal(true) + // success: false tracks the non-zero exit code; cancellation semantics live in data.status. + expect(json.success).to.equal(false) expect(json.data).to.deep.include({event: 'cancelled', status: 'cancelled'}) }) }) diff --git a/test/commands/dream.test.ts b/test/commands/dream.test.ts index d2fbc42d0..543bb1ab4 100644 --- a/test/commands/dream.test.ts +++ b/test/commands/dream.test.ts @@ -336,7 +336,8 @@ describe('Dream Command', () => { const json = parseJsonOutput() expect(json.command).to.equal('dream') - expect(json.success).to.equal(true) + // success: false tracks the non-zero exit code; cancellation semantics live in data.status. + expect(json.success).to.equal(false) expect(json.data).to.deep.include({event: 'cancelled', status: 'cancelled'}) }) }) diff --git a/test/commands/query.test.ts b/test/commands/query.test.ts index ac5dee6ed..8446570f3 100644 --- a/test/commands/query.test.ts +++ b/test/commands/query.test.ts @@ -787,6 +787,8 @@ describe('Query Command', () => { const cancelled = lines.find((line) => line.data.status === 'cancelled') expect(cancelled).to.not.equal(undefined) expect(cancelled!.command).to.equal('query') + // success: false tracks the non-zero exit code; cancellation semantics live in data.status. + expect(cancelled!.success).to.equal(false) }) }) }) From acb2acc4097a3bc3e805695c7a464bb7ab8bbefc Mon Sep 17 00:00:00 2001 From: Bao Ha Date: Wed, 13 May 2026 16:18:31 +0700 Subject: [PATCH 16/21] fix: [ENG-2783] durable cancel idempotency via persistent history store MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous Nit-3 fix only looked up the cancelled status in the in-memory `completedTasks` map. That map is cleared by a setTimeout after `TASK_CLEANUP_GRACE_PERIOD_MS` (5s), so a cancel retry arriving after the grace window would still report "Task not found" even though the task was successfully cancelled — semantically wrong, since the task is in fact cancelled in the persistent history. Extend handleTaskCancel with a durable second fallback: when the task is gone from both this.tasks AND completedTasks, look it up by id in the persistent task history store (resolved via the client's project path). If the persisted status is 'cancelled', return success. Any other status (completed / error) or a missing entry still returns the structured "Task not found". The handler becomes async because store.getById is async; the existing sync test invocations get `await` added. No production caller depended on the sync return — `transport.onRequest` supports both shapes. 2 new tests in task-router.test.ts: - 'returns success on a retry after completedTasks has aged out — durable via persistent history store' - 'still returns Task not found when the history store has a non-cancelled status (e.g. completed)' The original in-memory fast path is preserved so the common case (retry within 5s) avoids a disk read. --- src/server/infra/process/task-router.ts | 34 ++++++-- test/unit/infra/process/task-router.test.ts | 83 +++++++++++++++---- .../infra/process/transport-handlers.test.ts | 29 +++---- 3 files changed, 112 insertions(+), 34 deletions(-) diff --git a/src/server/infra/process/task-router.ts b/src/server/infra/process/task-router.ts index 1f52f4a83..5fb42cd4a 100644 --- a/src/server/infra/process/task-router.ts +++ b/src/server/infra/process/task-router.ts @@ -632,23 +632,45 @@ export class TaskRouter { } } - private handleTaskCancel(data: TaskCancelRequest, _clientId: string): TaskCancelResponse { + private async handleTaskCancel(data: TaskCancelRequest, clientId: string): Promise { const {taskId} = data transportLog(`Task cancel requested: ${taskId}`) const task = this.tasks.get(taskId) if (!task) { - // Idempotency: if the task already reached status: 'cancelled' on a - // prior cancel, return success so a retry (e.g. withDaemonRetry on a - // dropped response) isn't reported as a misleading "Task not found" - // failure. Any other terminal state (completed / error) — or a truly - // unknown taskId — still returns the structured error. + // Idempotency — fast path: if the task already reached status: + // 'cancelled' on a prior cancel, the in-memory completedTasks entry + // (within TASK_CLEANUP_GRACE_PERIOD_MS) lets us short-circuit without + // a disk read. const prior = this.completedTasks.get(taskId) if (prior?.task.status === 'cancelled') { return {success: true} } + // Idempotency — durable path: completedTasks is a cache and ages out + // after the cleanup grace period. The persistent task history store, + // populated by TaskHistoryHook.onTaskCancelled, is the long-lived + // source of truth. Consulting it lets late retries (and retries + // across daemon restarts) still report success rather than a + // misleading "Task not found". + if (this.getTaskHistoryStore !== undefined) { + const projectPath = this.resolveClientProjectPath?.(clientId) + if (projectPath !== undefined) { + try { + const store = this.getTaskHistoryStore(projectPath) + const entry = await store.getById(taskId) + if (entry?.status === 'cancelled') { + return {success: true} + } + } catch (error) { + transportLog( + `handleTaskCancel: history-store lookup failed for ${taskId}: ${error instanceof Error ? error.message : String(error)}`, + ) + } + } + } + return {error: 'Task not found', success: false} } diff --git a/test/unit/infra/process/task-router.test.ts b/test/unit/infra/process/task-router.test.ts index 468e38c68..595aac18f 100644 --- a/test/unit/infra/process/task-router.test.ts +++ b/test/unit/infra/process/task-router.test.ts @@ -810,10 +810,10 @@ describe('TaskRouter', () => { ;(transportHelper.transport.sendTo as SinonStub).resetHistory() }) - it('should forward cancel to agent when agent connected', () => { + it('should forward cancel to agent when agent connected', async () => { const handler = transportHelper.requestHandlers.get(TransportTaskEventNames.CANCEL) - const result = handler!({taskId}, 'client-1') + const result = await handler!({taskId}, 'client-1') expect(result).to.deep.equal({success: true}) expect((transportHelper.transport.sendTo as SinonStub).calledWith( @@ -823,12 +823,12 @@ describe('TaskRouter', () => { )).to.be.true }) - it('should cancel task locally when no agent connected', () => { + it('should cancel task locally when no agent connected', async () => { getAgentForProject.resetBehavior() const handler = transportHelper.requestHandlers.get(TransportTaskEventNames.CANCEL) - const result = handler!({taskId}, 'client-1') + const result = await handler!({taskId}, 'client-1') expect(result).to.deep.equal({success: true}) @@ -843,15 +843,15 @@ describe('TaskRouter', () => { expect(router.getTasksForProject('/app')).to.have.lengthOf(0) }) - it('should return error for unknown taskId', () => { + it('should return error for unknown taskId', async () => { const handler = transportHelper.requestHandlers.get(TransportTaskEventNames.CANCEL) - const result = handler!({taskId: 'nonexistent'}, 'client-1') + const result = await handler!({taskId: 'nonexistent'}, 'client-1') expect(result).to.deep.equal({error: 'Task not found', success: false}) }) - it('returns success on a retry after the task was already cancelled (idempotent)', () => { + it('returns success on a retry after the task was already cancelled (idempotent, in-memory fast path)', async () => { // First cancel succeeds and moves the task into completedTasks via // handleTaskCancelled. A retry (e.g. due to a dropped response in // withDaemonRetry) sees the task gone from this.tasks but still @@ -861,17 +861,72 @@ describe('TaskRouter', () => { cancelledHandler!({taskId}, 'agent-1') const cancelHandler = transportHelper.requestHandlers.get(TransportTaskEventNames.CANCEL) - const result = cancelHandler!({taskId}, 'client-1') + const result = await cancelHandler!({taskId}, 'client-1') expect(result).to.deep.equal({success: true}) }) + it('returns success on a retry after completedTasks has aged out — durable via persistent history store', async () => { + // Simulate the after-grace-period case: the task entry has been + // removed from in-memory completedTasks by the cleanup setTimeout, + // but the persistent task history store still has it with status: + // 'cancelled' (written by TaskHistoryHook.onTaskCancelled). + // + // The daemon must consult the store and still return success so + // longer-delayed retries don't get a misleading "Task not found". + const getStoreStub = sandbox.stub().returns({ + getById: sandbox.stub().resolves({status: 'cancelled', taskId}), + }) + const resolveClientStub = sandbox.stub().returns('/app') + + const routerWithStore = new TaskRouter({ + agentPool, + getAgentForProject, + getTaskHistoryStore: getStoreStub, + projectRegistry, + projectRouter, + resolveClientProjectPath: resolveClientStub, + transport: transportHelper.transport, + }) + routerWithStore.setup() + + const cancelHandler = transportHelper.requestHandlers.get(TransportTaskEventNames.CANCEL) + const result = await cancelHandler!({taskId: 'aged-out-task'}, 'client-1') + + expect(result).to.deep.equal({success: true}) + expect(getStoreStub.calledOnceWithExactly('/app')).to.equal(true) + }) + + it('still returns Task not found when the history store has a non-cancelled status (e.g. completed)', async () => { + // Negative case: only `status: cancelled` history entries map to + // idempotent success. A completed-then-deleted task should still + // surface as "Task not found" so the user sees a real signal. + const getStoreStub = sandbox.stub().returns({ + getById: sandbox.stub().resolves({status: 'completed', taskId}), + }) + + new TaskRouter({ + agentPool, + getAgentForProject, + getTaskHistoryStore: getStoreStub, + projectRegistry, + projectRouter, + resolveClientProjectPath: sandbox.stub().returns('/app'), + transport: transportHelper.transport, + }).setup() + + const cancelHandler = transportHelper.requestHandlers.get(TransportTaskEventNames.CANCEL) + const result = await cancelHandler!({taskId: 'completed-task'}, 'client-1') + + expect(result).to.deep.equal({error: 'Task not found', success: false}) + }) + describe('queued task cancellation (T1.3)', () => { - it('cancels a queued task directly via agentPool, broadcasts cancelled, never forwards to agent', () => { + it('cancels a queued task directly via agentPool, broadcasts cancelled, never forwards to agent', async () => { agentPool.cancelQueuedTask.returns(true) const handler = transportHelper.requestHandlers.get(TransportTaskEventNames.CANCEL) - const result = handler!({taskId}, 'client-1') + const result = await handler!({taskId}, 'client-1') expect(result).to.deep.equal({success: true}) expect(agentPool.cancelQueuedTask.calledOnceWithExactly(taskId)).to.equal(true) @@ -893,20 +948,20 @@ describe('TaskRouter', () => { expect(router.getTasksForProject('/app')).to.have.lengthOf(0) }) - it('does NOT call agentPool.notifyTaskCompleted for queued cancel (the task never occupied a pool slot)', () => { + it('does NOT call agentPool.notifyTaskCompleted for queued cancel (the task never occupied a pool slot)', async () => { agentPool.cancelQueuedTask.returns(true) const handler = transportHelper.requestHandlers.get(TransportTaskEventNames.CANCEL) - handler!({taskId}, 'client-1') + await handler!({taskId}, 'client-1') expect(agentPool.notifyTaskCompleted.called).to.equal(false) }) - it('falls through to forward-to-agent when cancelQueuedTask returns false (task is mid-execution)', () => { + it('falls through to forward-to-agent when cancelQueuedTask returns false (task is mid-execution)', async () => { agentPool.cancelQueuedTask.returns(false) const handler = transportHelper.requestHandlers.get(TransportTaskEventNames.CANCEL) - const result = handler!({taskId}, 'client-1') + const result = await handler!({taskId}, 'client-1') expect(result).to.deep.equal({success: true}) expect((transportHelper.transport.sendTo as SinonStub).calledWith( diff --git a/test/unit/infra/process/transport-handlers.test.ts b/test/unit/infra/process/transport-handlers.test.ts index 78525fb26..c370b0c67 100644 --- a/test/unit/infra/process/transport-handlers.test.ts +++ b/test/unit/infra/process/transport-handlers.test.ts @@ -552,7 +552,7 @@ describe('TransportHandlers', () => { }) describe('Task Cancellation', () => { - it('should forward cancel to agent when connected', () => { + it('should forward cancel to agent when connected', async () => { const createHandler = requestHandlers.get(TransportTaskEventNames.CREATE) registerAgentWithStatus('agent-001') @@ -560,7 +560,7 @@ describe('TransportHandlers', () => { createHandler!({content: 'Cancel test', taskId, type: 'curate'}, 'client-001') const cancelHandler = requestHandlers.get(TransportTaskEventNames.CANCEL) - const result = cancelHandler!({taskId}, 'client-001') + const result = await cancelHandler!({taskId}, 'client-001') expect(result).to.deep.equal({success: true}) expect( @@ -570,7 +570,7 @@ describe('TransportHandlers', () => { ).to.be.true }) - it('should cancel task locally when no agent registered for project', () => { + it('should cancel task locally when no agent registered for project', async () => { const createHandler = requestHandlers.get(TransportTaskEventNames.CREATE) // Create task WITHOUT registering agent first @@ -580,7 +580,7 @@ describe('TransportHandlers', () => { // Cancel should succeed — task is tracked, cancelled locally (no agent to forward to) const cancelHandler = requestHandlers.get(TransportTaskEventNames.CANCEL) - const result = cancelHandler!({taskId}, 'client-001') + const result = await cancelHandler!({taskId}, 'client-001') expect(result).to.deep.equal({success: true}) @@ -589,9 +589,9 @@ describe('TransportHandlers', () => { .to.be.true }) - it('should return error for non-existent task', () => { + it('should return error for non-existent task', async () => { const cancelHandler = requestHandlers.get(TransportTaskEventNames.CANCEL) - const result = cancelHandler!({taskId: 'non-existent-task'}, 'client-001') + const result = await cancelHandler!({taskId: 'non-existent-task'}, 'client-001') expect(result).to.deep.equal({error: 'Task not found', success: false}) }) @@ -742,7 +742,7 @@ describe('TransportHandlers', () => { ).to.be.true }) - it('should clear tasks map after agent disconnect', () => { + it('should clear tasks map after agent disconnect', async () => { const createHandler = requestHandlers.get(TransportTaskEventNames.CREATE) registerAgentWithStatus('agent-001') @@ -757,12 +757,12 @@ describe('TransportHandlers', () => { // Try to cancel the old task - should fail (not found) const cancelHandler = requestHandlers.get(TransportTaskEventNames.CANCEL) - const result = cancelHandler!({taskId: 'old-task-id'}, 'client-001') + const result = await cancelHandler!({taskId: 'old-task-id'}, 'client-001') expect(result).to.deep.equal({error: 'Task not found', success: false}) }) - it('should not affect non-agent client disconnections', () => { + it('should not affect non-agent client disconnections', async () => { const createHandler = requestHandlers.get(TransportTaskEventNames.CREATE) registerAgentWithStatus('agent-001') @@ -775,7 +775,7 @@ describe('TransportHandlers', () => { // Agent should still be connected, task should still exist // Cancel should still work const cancelHandler = requestHandlers.get(TransportTaskEventNames.CANCEL) - const result = cancelHandler!({taskId}, 'client-002') + const result = await cancelHandler!({taskId}, 'client-002') expect(result).to.deep.equal({success: true}) }) @@ -1082,7 +1082,7 @@ describe('TransportHandlers', () => { }) describe('Cleanup', () => { - it('should clear tasks and agent on cleanup()', () => { + it('should clear tasks and agent on cleanup()', async () => { const createHandler = requestHandlers.get(TransportTaskEventNames.CREATE) registerAgentWithStatus('agent-001') @@ -1093,7 +1093,7 @@ describe('TransportHandlers', () => { // After cleanup, cancel should fail (no tasks) const cancelHandler = requestHandlers.get(TransportTaskEventNames.CANCEL) - const result = cancelHandler!({taskId: 'any-task'}, 'client-001') + const result = await cancelHandler!({taskId: 'any-task'}, 'client-001') expect(result).to.deep.equal({error: 'Task not found', success: false}) // Restart should fail (no agent) @@ -1493,7 +1493,7 @@ describe('TransportHandlers', () => { } }) - it('should handle rapid create/cancel cycles', () => { + it('should handle rapid create/cancel cycles', async () => { const createHandler = requestHandlers.get(TransportTaskEventNames.CREATE) const cancelHandler = requestHandlers.get(TransportTaskEventNames.CANCEL) registerAgentWithStatus('agent-001') @@ -1501,7 +1501,8 @@ describe('TransportHandlers', () => { for (let i = 0; i < 20; i++) { const taskId = randomUUID() createHandler!({content: `Rapid ${i}`, taskId, type: 'curate'}, 'client-001') - const cancelResult = cancelHandler!({taskId}, 'client-001') + // eslint-disable-next-line no-await-in-loop + const cancelResult = await cancelHandler!({taskId}, 'client-001') expect(cancelResult).to.deep.equal({success: true}) } }) From 45a0ff054c851ca0103ba852999acac2c73712da Mon Sep 17 00:00:00 2001 From: Bao Ha Date: Thu, 14 May 2026 10:08:45 +0700 Subject: [PATCH 17/21] fix: [ENG-2783] cancelTaskLocally cache miss + post-review nits MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - task-router: cancelTaskLocally now stamps status='cancelled' and routes through moveToCompleted so the in-memory fast-path idempotency catches sequential retries on the queued/no-agent cancel paths. Previously the retry raced the fire-and-forget durable hook write and could see a misleading 'Task not found'. Adds regression test. - transport: document TaskCancelResponse.success semantics across the four reply paths (queued, forwarded, idempotent retry, not-found), plus an inline note above the forward-to-agent branch in task-router. - oclif: inline the runCancelBranch wrapper in curate/dream/query — the three identical 9-line private methods are gone; the single use site in each command now calls runCancelBranchWithRetry directly. - task-client: introduce WaitForTaskClient = Pick so the SIGINT test stub satisfies the contract structurally without an 'as unknown as ITransportClient' cast; installSigintCancel narrowed to Pick<..., 'request'> for the same reason. - tui: expand JSDoc on use-cancel-running-task-keybind with a RESERVED CHORD WARNING so contributors know Ctrl+Q is globally intercepted whenever any non-terminal task exists in the tasks store. --- src/oclif/commands/curate/index.ts | 22 ++++++++----------- src/oclif/commands/dream.ts | 22 ++++++++----------- src/oclif/commands/query.ts | 22 ++++++++----------- src/oclif/lib/task-client.ts | 11 ++++++++-- src/server/infra/process/task-router.ts | 19 ++++++++++++++-- src/shared/transport/events/task-events.ts | 16 ++++++++++++++ .../hooks/use-cancel-running-task-keybind.ts | 15 +++++++++++++ test/unit/infra/process/task-router.test.ts | 16 ++++++++++++++ .../unit/oclif/lib/task-client-sigint.test.ts | 19 ++++++++-------- 9 files changed, 110 insertions(+), 52 deletions(-) diff --git a/src/oclif/commands/curate/index.ts b/src/oclif/commands/curate/index.ts index 948d05e57..f14ad4179 100644 --- a/src/oclif/commands/curate/index.ts +++ b/src/oclif/commands/curate/index.ts @@ -116,7 +116,15 @@ Bad examples: const format: 'json' | 'text' = flags.format ?? 'text' if (rawFlags.cancel) { - await this.runCancelBranch(rawFlags.cancel, format) + const ok = await runCancelBranchWithRetry({ + command: 'curate', + daemonClientOptions: this.getDaemonClientOptions(), + format, + log: (msg) => this.log(msg), + onTransportError: (error) => this.reportError(error, format), + taskId: rawFlags.cancel, + }) + if (!ok) this.exit(1) return } @@ -304,18 +312,6 @@ Bad examples: } } - private async runCancelBranch(taskId: string, format: 'json' | 'text'): Promise { - const success = await runCancelBranchWithRetry({ - command: 'curate', - daemonClientOptions: this.getDaemonClientOptions(), - format, - log: (msg) => this.log(msg), - onTransportError: (error) => this.reportError(error, format), - taskId, - }) - if (!success) this.exit(1) - } - private async submitTask(props: { client: ITransportClient content: string diff --git a/src/oclif/commands/dream.ts b/src/oclif/commands/dream.ts index bbd6706dc..cc62e3aaa 100644 --- a/src/oclif/commands/dream.ts +++ b/src/oclif/commands/dream.ts @@ -123,7 +123,15 @@ export default class Dream extends Command { const format = rawFlags.format === 'json' ? 'json' : 'text' if (rawFlags.cancel) { - await this.runCancelBranch(rawFlags.cancel, format) + const ok = await runCancelBranchWithRetry({ + command: 'dream', + daemonClientOptions: this.getDaemonClientOptions(), + format, + log: (msg) => this.log(msg), + onTransportError: (error) => this.reportError(error, format), + taskId: rawFlags.cancel, + }) + if (!ok) this.exit(1) return } @@ -199,18 +207,6 @@ export default class Dream extends Command { } } - private async runCancelBranch(taskId: string, format: 'json' | 'text'): Promise { - const success = await runCancelBranchWithRetry({ - command: 'dream', - daemonClientOptions: this.getDaemonClientOptions(), - format, - log: (msg) => this.log(msg), - onTransportError: (error) => this.reportError(error, format), - taskId, - }) - if (!success) this.exit(1) - } - private async runUndo(format: 'json' | 'text'): Promise { const projectRoot = resolveProject()?.projectRoot ?? process.cwd() // JSON mode: route sidecar warnings to a no-op so structured stdout diff --git a/src/oclif/commands/query.ts b/src/oclif/commands/query.ts index b9d22049e..6116ec14f 100644 --- a/src/oclif/commands/query.ts +++ b/src/oclif/commands/query.ts @@ -73,7 +73,15 @@ Bad: return } - await this.runCancelBranch(flags.cancel, format) + const ok = await runCancelBranchWithRetry({ + command: 'query', + daemonClientOptions: this.getDaemonClientOptions(), + format, + log: (msg) => this.log(msg), + onTransportError: (error) => this.reportError(error, format), + taskId: flags.cancel, + }) + if (!ok) this.exit(1) return } @@ -158,18 +166,6 @@ Bad: } } - private async runCancelBranch(taskId: string, format: 'json' | 'text'): Promise { - const success = await runCancelBranchWithRetry({ - command: 'query', - daemonClientOptions: this.getDaemonClientOptions(), - format, - log: (msg) => this.log(msg), - onTransportError: (error) => this.reportError(error, format), - taskId, - }) - if (!success) this.exit(1) - } - private async submitTask(props: { client: ITransportClient format: 'json' | 'text' diff --git a/src/oclif/lib/task-client.ts b/src/oclif/lib/task-client.ts index 8f5c683e5..892a2cd9c 100644 --- a/src/oclif/lib/task-client.ts +++ b/src/oclif/lib/task-client.ts @@ -85,10 +85,17 @@ export interface TaskErrorResult { toolCalls: ToolCallRecord[] } +/** + * Subset of ITransportClient used by waitForTaskCompletion + installSigintCancel. + * Narrow on purpose so test stubs can satisfy the contract structurally without + * an `as unknown as ITransportClient` cast. + */ +export type WaitForTaskClient = Pick + /** Options for waitForTaskCompletion */ export interface WaitForTaskOptions { /** Client to subscribe events on */ - client: ITransportClient + client: WaitForTaskClient /** Command name for JSON output */ command: string /** Output format */ @@ -173,7 +180,7 @@ export function formatToolDisplay(toolName: string, args: Record void { +function installSigintCancel(deps: {client: Pick; taskId: string}): () => void { let cancelRequested = false const onSigint = (): void => { diff --git a/src/server/infra/process/task-router.ts b/src/server/infra/process/task-router.ts index 5fb42cd4a..45c73ddcc 100644 --- a/src/server/infra/process/task-router.ts +++ b/src/server/infra/process/task-router.ts @@ -600,7 +600,15 @@ export class TaskRouter { {taskId}, task.clientId, ) - this.tasks.delete(taskId) + // Stamp status='cancelled' before moveToCompleted so the in-memory + // completedTasks cache stores a 'cancelled' entry — the fast-path + // idempotency check at handleTaskCancel reads `prior?.task.status`. + // Without this, sequential retries (queued-cancel + no-agent-cancel + // paths) would skip the in-memory cache entirely and have to win a + // race against the fire-and-forget notifyHooksCancelled before the + // durable history-store lookup could see the persisted status. + this.tasks.set(taskId, {...task, completedAt: Date.now(), status: 'cancelled'}) + this.moveToCompleted(taskId) this.notifyHooksCancelled(taskId, task).catch(() => {}) } @@ -685,7 +693,14 @@ export class TaskRouter { return {success: true} } - // If Agent connected for this task's project, forward cancel request + // If Agent connected for this task's project, forward cancel request. + // NOTE: `success: true` here means "queued for forward", NOT "agent + // confirmed cancellation". The agent may have already finished the task + // (race window between daemon dispatch and agent terminal event), in + // which case agent-cancel-listener returns false and emits no + // task:cancelled. Callers must rely on the terminal event broadcast, + // not this success flag, to know the final task state. See JSDoc on + // TaskCancelResponse for full semantics. const agentId = this.getAgentForProject(task.projectPath) if (agentId) { this.transport.sendTo(agentId, TransportTaskEventNames.CANCEL, {taskId}) diff --git a/src/shared/transport/events/task-events.ts b/src/shared/transport/events/task-events.ts index 5d09c19f6..116f2da46 100644 --- a/src/shared/transport/events/task-events.ts +++ b/src/shared/transport/events/task-events.ts @@ -41,6 +41,22 @@ export interface TaskCancelRequest { taskId: string } +/** + * Daemon's reply to a `task:cancel` request. + * + * Semantics of `success`: + * - `true` from a queued/local-cancel path → terminal `task:cancelled` has + * already been broadcast by the daemon; the task is fully cancelled. + * - `true` from the forward-to-agent path → the cancel was forwarded to a + * live agent, NOT confirmed. The agent may have already finished the task + * (race), in which case no `task:cancelled` follows; the wait loop must + * still rely on the actual terminal event broadcast (`task:cancelled` / + * `task:completed` / `task:error`) to determine the final state. + * - `true` from an idempotent retry → the task was previously observed as + * `status: 'cancelled'` (in-memory cache or persistent history store). + * - `false` with `error: 'Task not found'` → daemon has no record of the + * task in any tier (active / completed-cache / persistent history). + */ export interface TaskCancelResponse { error?: string success: boolean diff --git a/src/tui/features/tasks/hooks/use-cancel-running-task-keybind.ts b/src/tui/features/tasks/hooks/use-cancel-running-task-keybind.ts index 691abdd60..c146d1e43 100644 --- a/src/tui/features/tasks/hooks/use-cancel-running-task-keybind.ts +++ b/src/tui/features/tasks/hooks/use-cancel-running-task-keybind.ts @@ -8,6 +8,21 @@ * Why scoped here instead of inside curate-flow / query-flow: those components * unmount as soon as the daemon acks task:create (< 100ms), so they cannot * own a keybind that needs to be active for the entire task lifetime. + * + * RESERVED CHORD WARNING (read before binding Ctrl+Q anywhere else in the TUI): + * - This keybind is mounted globally via `app-providers.tsx → CancelKeybindInitializer`, + * so as soon as ANY non-terminal task exists in the tasks store, Ctrl+Q is + * intercepted everywhere — text inputs, REPL prompt, search box, modals, etc. + * - DO NOT bind Ctrl+Q to any other action while a curate/query/dream task can + * be in flight; the global cancel will eat the chord first. + * - The binding targets `selectCancelTargetTaskId`'s "most recently created + * non-terminal task." With multiple in-flight tasks, Ctrl+Q always cancels + * the most recent one, not the one currently focused in the UI. + * - There is no confirmation step today — the cancel network round-trip starts + * the instant the chord lands. If a real-world conflict surfaces (user + * accidentally cancelling), the recommended follow-up is to mirror the SIGINT + * double-tap pattern in `installSigintCancel` (first Ctrl+Q arms, second + * within Ns fires). */ import {useInput} from 'ink' diff --git a/test/unit/infra/process/task-router.test.ts b/test/unit/infra/process/task-router.test.ts index 595aac18f..49ae63dc2 100644 --- a/test/unit/infra/process/task-router.test.ts +++ b/test/unit/infra/process/task-router.test.ts @@ -970,6 +970,22 @@ describe('TaskRouter', () => { {taskId}, )).to.be.true }) + + it('returns success on a sequential retry after queued cancel — in-memory fast path', async () => { + // Regression guard: cancelTaskLocally must seed completedTasks with a + // 'cancelled' entry so the fast-path idempotency check at handleTaskCancel + // catches a sequential retry. Without that seeding, the retry would race + // the fire-and-forget notifyHooksCancelled to the durable history store + // and could return a misleading "Task not found". + agentPool.cancelQueuedTask.returns(true) + const handler = transportHelper.requestHandlers.get(TransportTaskEventNames.CANCEL) + + const first = await handler!({taskId}, 'client-1') + const second = await handler!({taskId}, 'client-1') + + expect(first).to.deep.equal({success: true}) + expect(second).to.deep.equal({success: true}) + }) }) }) diff --git a/test/unit/oclif/lib/task-client-sigint.test.ts b/test/unit/oclif/lib/task-client-sigint.test.ts index 18fee3938..34b1d631e 100644 --- a/test/unit/oclif/lib/task-client-sigint.test.ts +++ b/test/unit/oclif/lib/task-client-sigint.test.ts @@ -1,17 +1,17 @@ -import type {ITransportClient} from '@campfirein/brv-transport-client' - import {expect} from 'chai' import {createSandbox, type SinonSandbox, type SinonStub} from 'sinon' -import {waitForTaskCompletion} from '../../../../src/oclif/lib/task-client.js' +import {type WaitForTaskClient, waitForTaskCompletion} from '../../../../src/oclif/lib/task-client.js' type EventHandler = (data: unknown) => void /** - * Build a TransportClient stub that exposes: + * Build a WaitForTaskClient stub that exposes: * - eventHandlers map keyed by event name with the registered listener * - on/onStateChange returning unsubscribe stubs (counted for cleanup checks) * - request stub for verifying the cancel emission + * + * Stub conforms to the narrow `WaitForTaskClient` contract — no cast needed. */ function makeStubClient(sandbox: SinonSandbox) { const eventHandlers = new Map() @@ -28,12 +28,13 @@ function makeStubClient(sandbox: SinonSandbox) { const onStateChangeStub = sandbox.stub().returns(stateUnsub) unsubscribeStubs.push(stateUnsub) - const client = { + const requestStub = sandbox.stub() + + const client: WaitForTaskClient = { on: onStub, onStateChange: onStateChangeStub, - request: sandbox.stub(), - requestWithAck: sandbox.stub(), - } as unknown as ITransportClient + request: requestStub, + } return { client, @@ -41,7 +42,7 @@ function makeStubClient(sandbox: SinonSandbox) { eventHandlers.get(event)?.(data) }, onStub, - requestStub: client.request as SinonStub, + requestStub, unsubscribeStubs, } } From 06b9c0750ef9133bfe6acdf533ceee0dcc813ca9 Mon Sep 17 00:00:00 2001 From: Bao Ha Date: Wed, 20 May 2026 11:00:20 +0700 Subject: [PATCH 18/21] test: [ENG-2783] drop stale timeoutMs from SIGINT test options The waitForTaskCompletion options interface lost timeoutMs when the per-call setTimeout was replaced with a heartbeat-driven stale check, but this test file kept the field, breaking the suite with TS2353. --- test/unit/oclif/lib/task-client-sigint.test.ts | 8 -------- 1 file changed, 8 deletions(-) diff --git a/test/unit/oclif/lib/task-client-sigint.test.ts b/test/unit/oclif/lib/task-client-sigint.test.ts index 34b1d631e..d88b6e751 100644 --- a/test/unit/oclif/lib/task-client-sigint.test.ts +++ b/test/unit/oclif/lib/task-client-sigint.test.ts @@ -90,7 +90,6 @@ describe('waitForTaskCompletion — SIGINT cancel handling (T2.5)', () => { onCompleted() {}, onError() {}, taskId: 'task-1', - timeoutMs: 5000, }, () => {}, ) @@ -117,7 +116,6 @@ describe('waitForTaskCompletion — SIGINT cancel handling (T2.5)', () => { onCompleted() {}, onError() {}, taskId: 'task-1', - timeoutMs: 5000, }, () => {}, ) @@ -144,7 +142,6 @@ describe('waitForTaskCompletion — SIGINT cancel handling (T2.5)', () => { onCompleted() {}, onError() {}, taskId: 'task-1', - timeoutMs: 5000, }, () => {}, ) @@ -175,7 +172,6 @@ describe('waitForTaskCompletion — SIGINT cancel handling (T2.5)', () => { onCompleted() {}, onError() {}, taskId: 'task-1', - timeoutMs: 5000, }, () => {}, ) @@ -204,7 +200,6 @@ describe('waitForTaskCompletion — SIGINT cancel handling (T2.5)', () => { onCompleted() {}, onError() {}, taskId: 'task-1', - timeoutMs: 5000, }, () => {}, ) @@ -231,7 +226,6 @@ describe('waitForTaskCompletion — SIGINT cancel handling (T2.5)', () => { onCompleted() {}, onError() {}, taskId: 'task-1', - timeoutMs: 5000, }, () => {}, ) @@ -256,7 +250,6 @@ describe('waitForTaskCompletion — SIGINT cancel handling (T2.5)', () => { onCompleted() {}, onError() {}, taskId: 'task-1', - timeoutMs: 5000, }, () => {}, ) @@ -290,7 +283,6 @@ describe('waitForTaskCompletion — SIGINT cancel handling (T2.5)', () => { onCompleted() {}, onError() {}, taskId: 'task-1', - timeoutMs: 5000, }, () => {}, ) From 5a559ff069796b19f026774bbcbb41036dceeffc Mon Sep 17 00:00:00 2001 From: ncnthien Date: Fri, 22 May 2026 15:04:24 +0700 Subject: [PATCH 19/21] feat: [ENG-2784] add WebUI cancel button to task list rows and detail header --- src/webui/features/tasks/api/cancel-task.ts | 29 ++++++++ .../tasks/components/task-detail-header.tsx | 28 ++++++-- .../tasks/components/task-detail-view.tsx | 13 +++- .../tasks/components/task-list-table.tsx | 37 +++++++++- .../tasks/components/task-list-view.tsx | 32 +++++++++ .../features/tasks/utils/row-action-kind.ts | 9 +++ .../features/tasks/api/cancel-task.test.ts | 69 +++++++++++++++++++ .../tasks/utils/row-action-kind.test.ts | 25 +++++++ 8 files changed, 234 insertions(+), 8 deletions(-) create mode 100644 src/webui/features/tasks/api/cancel-task.ts create mode 100644 src/webui/features/tasks/utils/row-action-kind.ts create mode 100644 test/unit/webui/features/tasks/api/cancel-task.test.ts create mode 100644 test/unit/webui/features/tasks/utils/row-action-kind.test.ts diff --git a/src/webui/features/tasks/api/cancel-task.ts b/src/webui/features/tasks/api/cancel-task.ts new file mode 100644 index 000000000..174414484 --- /dev/null +++ b/src/webui/features/tasks/api/cancel-task.ts @@ -0,0 +1,29 @@ +import {useMutation} from '@tanstack/react-query' + +import type {MutationConfig} from '../../../lib/react-query' + +import { + type TaskCancelRequest, + type TaskCancelResponse, + TaskEvents, +} from '../../../../shared/transport/events/task-events' +import {useTransportStore} from '../../../stores/transport-store' + +export const cancelTask = async (payload: TaskCancelRequest): Promise => { + const {apiClient} = useTransportStore.getState() + if (!apiClient) throw new Error('Not connected') + + const response = await apiClient.request(TaskEvents.CANCEL, payload) + if (!response.success) throw new Error(response.error ?? 'Cancel failed') + return response +} + +type UseCancelTaskOptions = { + mutationConfig?: MutationConfig +} + +export const useCancelTask = ({mutationConfig}: UseCancelTaskOptions = {}) => + useMutation({ + ...mutationConfig, + mutationFn: cancelTask, + }) diff --git a/src/webui/features/tasks/components/task-detail-header.tsx b/src/webui/features/tasks/components/task-detail-header.tsx index 293118684..348949e25 100644 --- a/src/webui/features/tasks/components/task-detail-header.tsx +++ b/src/webui/features/tasks/components/task-detail-header.tsx @@ -1,6 +1,7 @@ import {Button} from '@campfirein/byterover-packages/components/button' import {Tooltip, TooltipContent, TooltipTrigger} from '@campfirein/byterover-packages/components/tooltip' import {cn} from '@campfirein/byterover-packages/lib/utils' +import {CircleStop} from 'lucide-react' import {toast} from 'sonner' import type {StoredTask} from '../types/stored-task' @@ -18,8 +19,16 @@ const STATUS_VERB: Record = { started: 'started', } -export function DetailHeader({now, task}: {now: number; task: StoredTask}) { +interface DetailHeaderProps { + cancelling: boolean + now: number + onCancel: (taskId: string) => void + task: StoredTask +} + +export function DetailHeader({cancelling, now, onCancel, task}: DetailHeaderProps) { const isTerminal = isTerminalStatus(task.status) + const isActive = isActiveStatus(task.status) const elapsed = elapsedMs(task, now) const referenceTime = task.startedAt ?? task.createdAt const verb = STATUS_VERB[task.status] @@ -42,11 +51,22 @@ export function DetailHeader({now, task}: {now: number; task: StoredTask}) { {verb} {formatRelative(referenceTime, now)} ago - + {elapsedLabel} {formatDuration(elapsed)} + {isActive && ( + + )} ) diff --git a/src/webui/features/tasks/components/task-detail-view.tsx b/src/webui/features/tasks/components/task-detail-view.tsx index 0a07bb1ba..b00a762a4 100644 --- a/src/webui/features/tasks/components/task-detail-view.tsx +++ b/src/webui/features/tasks/components/task-detail-view.tsx @@ -1,9 +1,12 @@ import type {ComponentRef} from 'react' +import {toast} from 'sonner' + import type {StoredTask} from '../types/stored-task' import {TourTaskBanner, TourTaskContinueCta} from '../../onboarding/components/tour-task-banner' import {useOnboardingStore} from '../../onboarding/stores/onboarding-store' +import {useCancelTask} from '../api/cancel-task' import {useGetTaskDetail} from '../api/get-task' import {useStickToBottom} from '../hooks/use-stick-to-bottom' import {useTickingNow} from '../hooks/use-ticking-now' @@ -41,6 +44,14 @@ export function TaskDetailView({taskId}: TaskDetailViewProps) { const tourTaskId = useOnboardingStore((s) => s.tourTaskId) const isTourTask = tourTaskId === taskId + const cancelMutation = useCancelTask() + const handleCancel = (id: string) => { + cancelMutation.mutate( + {taskId: id}, + {onError: (err) => toast.error(err.message)}, + ) + } + const lastReasoning = task?.reasoningContents?.at(-1) const {onScroll, ref: scrollRef} = useStickToBottom>( [ @@ -80,7 +91,7 @@ export function TaskDetailView({taskId}: TaskDetailViewProps) { return (
- +
diff --git a/src/webui/features/tasks/components/task-list-table.tsx b/src/webui/features/tasks/components/task-list-table.tsx index e1ea597d7..03d7e8f60 100644 --- a/src/webui/features/tasks/components/task-list-table.tsx +++ b/src/webui/features/tasks/components/task-list-table.tsx @@ -10,7 +10,7 @@ import { } from '@campfirein/byterover-packages/components/table' import {Tooltip, TooltipContent, TooltipTrigger} from '@campfirein/byterover-packages/components/tooltip' import {cn} from '@campfirein/byterover-packages/lib/utils' -import {Trash2} from 'lucide-react' +import {CircleStop, Trash2} from 'lucide-react' import type {StatusFilter} from '../stores/task-store' import type {StoredTask} from '../types/stored-task' @@ -19,6 +19,7 @@ import {getCurrentActivity} from '../utils/current-activity' import {formatProviderModel} from '../utils/format-provider-model' import {formatDuration, formatRelative, formatTimeOfDay, shortTaskId} from '../utils/format-time' import {isInterrupted} from '../utils/is-interrupted' +import {rowActionKind} from '../utils/row-action-kind' import {displayTaskType, isTerminalStatus} from '../utils/task-status' import {StatusPill} from './status-pill' import {NoMatchState} from './task-list-empty' @@ -46,8 +47,10 @@ function durationOf(task: StoredTask, now: number): string { interface TaskTableProps { allSelected: boolean + cancellingIds: Set filtered: StoredTask[] now: number + onCancel: (taskId: string) => void onClearSearch: () => void onDelete: (taskId: string) => void onRowClick: (taskId: string) => void @@ -61,8 +64,10 @@ interface TaskTableProps { export function TaskTable({ allSelected, + cancellingIds, filtered, now, + onCancel, onClearSearch, onDelete, onRowClick, @@ -100,9 +105,11 @@ export function TaskTable({ ) : ( filtered.map((task) => ( void onDelete: (taskId: string) => void onRowClick: (taskId: string) => void onToggleSelect: (taskId: string) => void @@ -137,6 +148,7 @@ function TaskRow({ const isRunning = !terminal const interrupted = isInterrupted(task) const activity = getCurrentActivity(task) + const actionKind = rowActionKind(task.status) const row = ( event.stopPropagation()}> - {terminal && onDelete(task.taskId)} />} + {actionKind === 'delete' ? ( + onDelete(task.taskId)} /> + ) : ( + onCancel(task.taskId)} /> + )} ) @@ -231,7 +247,7 @@ function ProviderChip({model, provider, providerName}: {model?: string; provider ) } -function RowAction({onClick}: {onClick: () => void}) { +function DeleteRowAction({onClick}: {onClick: () => void}) { return ( + ) +} + function Checkbox({checked, onChange}: {checked: boolean; onChange: () => void}) { return ( >(new Set()) + const [cancellingIds, setCancellingIds] = useState>(new Set()) const [composer, setComposer] = useState<{ initialContent?: string initialType?: ComposerType @@ -239,6 +242,33 @@ export function TaskListView() { ) } + const handleCancel = (taskId: string) => { + setCancellingIds((prev) => { + const next = new Set(prev) + next.add(taskId) + return next + }) + const clear = () => { + setCancellingIds((prev) => { + if (!prev.has(taskId)) return prev + const next = new Set(prev) + next.delete(taskId) + return next + }) + } + + cancelMutation.mutate( + {taskId}, + { + onError(err) { + toast.error(err.message) + clear() + }, + onSuccess: clear, + }, + ) + } + const deleteSelected = () => { const eligibleIds = [...selectedIds].filter((id) => { const task = taskMap.get(id) @@ -357,8 +387,10 @@ export function TaskListView() { ) : ( setSearchQuery('')} onDelete={handleDelete} onRowClick={openTask} diff --git a/src/webui/features/tasks/utils/row-action-kind.ts b/src/webui/features/tasks/utils/row-action-kind.ts new file mode 100644 index 000000000..2b0d64228 --- /dev/null +++ b/src/webui/features/tasks/utils/row-action-kind.ts @@ -0,0 +1,9 @@ +import type {TaskListItemStatus} from '../../../../shared/transport/events/task-events' + +import {isTerminalStatus} from './task-status' + +export type RowActionKind = 'cancel' | 'delete' + +export function rowActionKind(status: TaskListItemStatus): RowActionKind { + return isTerminalStatus(status) ? 'delete' : 'cancel' +} diff --git a/test/unit/webui/features/tasks/api/cancel-task.test.ts b/test/unit/webui/features/tasks/api/cancel-task.test.ts new file mode 100644 index 000000000..4260c86fb --- /dev/null +++ b/test/unit/webui/features/tasks/api/cancel-task.test.ts @@ -0,0 +1,69 @@ +import {expect} from 'chai' +import {createSandbox, type SinonSandbox, type SinonStub} from 'sinon' + +import type {BrvApiClient} from '../../../../../../src/webui/lib/api-client.js' + +import {TaskEvents} from '../../../../../../src/shared/transport/events/task-events.js' +import {cancelTask} from '../../../../../../src/webui/features/tasks/api/cancel-task.js' +import {useTransportStore} from '../../../../../../src/webui/stores/transport-store.js' + +describe('cancelTask', () => { + let sandbox: SinonSandbox + let request: SinonStub + + beforeEach(() => { + sandbox = createSandbox() + request = sandbox.stub() + useTransportStore.setState({ + apiClient: {on: sandbox.stub(), request} as unknown as BrvApiClient, + }) + }) + + afterEach(() => { + sandbox.restore() + useTransportStore.setState({apiClient: null}) + }) + + it('emits task:cancel with the taskId payload', async () => { + request.resolves({success: true}) + await cancelTask({taskId: 'tsk-1'}) + expect(request.firstCall.args[0]).to.equal(TaskEvents.CANCEL) + expect(request.firstCall.args[1]).to.deep.equal({taskId: 'tsk-1'}) + }) + + it('resolves with the daemon response on success', async () => { + request.resolves({success: true}) + const result = await cancelTask({taskId: 'tsk-1'}) + expect(result).to.deep.equal({success: true}) + }) + + it('throws when the daemon returns success: false', async () => { + request.resolves({error: 'Task not found', success: false}) + try { + await cancelTask({taskId: 'tsk-1'}) + expect.fail('expected to throw') + } catch (error) { + expect((error as Error).message).to.equal('Task not found') + } + }) + + it('falls back to "Cancel failed" when success: false has no error string', async () => { + request.resolves({success: false}) + try { + await cancelTask({taskId: 'tsk-1'}) + expect.fail('expected to throw') + } catch (error) { + expect((error as Error).message).to.equal('Cancel failed') + } + }) + + it('throws when not connected to the daemon', async () => { + useTransportStore.setState({apiClient: null}) + try { + await cancelTask({taskId: 'tsk-1'}) + expect.fail('expected to throw') + } catch (error) { + expect((error as Error).message).to.equal('Not connected') + } + }) +}) diff --git a/test/unit/webui/features/tasks/utils/row-action-kind.test.ts b/test/unit/webui/features/tasks/utils/row-action-kind.test.ts new file mode 100644 index 000000000..62624e612 --- /dev/null +++ b/test/unit/webui/features/tasks/utils/row-action-kind.test.ts @@ -0,0 +1,25 @@ +import {expect} from 'chai' + +import {rowActionKind} from '../../../../../../src/webui/features/tasks/utils/row-action-kind.js' + +describe('rowActionKind', () => { + it('returns "cancel" for created tasks (running)', () => { + expect(rowActionKind('created')).to.equal('cancel') + }) + + it('returns "cancel" for started tasks (running)', () => { + expect(rowActionKind('started')).to.equal('cancel') + }) + + it('returns "delete" for completed tasks (terminal)', () => { + expect(rowActionKind('completed')).to.equal('delete') + }) + + it('returns "delete" for error tasks (terminal)', () => { + expect(rowActionKind('error')).to.equal('delete') + }) + + it('returns "delete" for cancelled tasks (terminal)', () => { + expect(rowActionKind('cancelled')).to.equal('delete') + }) +}) From 98c0eb36a11f4aea03bfb75f9723f6d724544d5d Mon Sep 17 00:00:00 2001 From: ncnthien Date: Fri, 22 May 2026 15:17:21 +0700 Subject: [PATCH 20/21] feat: [ENG-2784] unify cancel state across list and detail, add pending feedback --- .../tasks/components/task-detail-header.tsx | 6 +++--- .../tasks/components/task-detail-view.tsx | 17 ++++------------- .../tasks/components/task-list-table.tsx | 12 ++++++------ .../tasks/components/task-list-view.tsx | 8 +++++++- 4 files changed, 20 insertions(+), 23 deletions(-) diff --git a/src/webui/features/tasks/components/task-detail-header.tsx b/src/webui/features/tasks/components/task-detail-header.tsx index 348949e25..360b74a69 100644 --- a/src/webui/features/tasks/components/task-detail-header.tsx +++ b/src/webui/features/tasks/components/task-detail-header.tsx @@ -1,7 +1,7 @@ import {Button} from '@campfirein/byterover-packages/components/button' import {Tooltip, TooltipContent, TooltipTrigger} from '@campfirein/byterover-packages/components/tooltip' import {cn} from '@campfirein/byterover-packages/lib/utils' -import {CircleStop} from 'lucide-react' +import {CircleStop, LoaderCircle} from 'lucide-react' import {toast} from 'sonner' import type {StoredTask} from '../types/stored-task' @@ -63,8 +63,8 @@ export function DetailHeader({cancelling, now, onCancel, task}: DetailHeaderProp size="xs" variant="outline" > - - Cancel + {cancelling ? : } + {cancelling ? 'Cancelling…' : 'Cancel'} )}
diff --git a/src/webui/features/tasks/components/task-detail-view.tsx b/src/webui/features/tasks/components/task-detail-view.tsx index b00a762a4..9f21f07b9 100644 --- a/src/webui/features/tasks/components/task-detail-view.tsx +++ b/src/webui/features/tasks/components/task-detail-view.tsx @@ -1,12 +1,9 @@ import type {ComponentRef} from 'react' -import {toast} from 'sonner' - import type {StoredTask} from '../types/stored-task' import {TourTaskBanner, TourTaskContinueCta} from '../../onboarding/components/tour-task-banner' import {useOnboardingStore} from '../../onboarding/stores/onboarding-store' -import {useCancelTask} from '../api/cancel-task' import {useGetTaskDetail} from '../api/get-task' import {useStickToBottom} from '../hooks/use-stick-to-bottom' import {useTickingNow} from '../hooks/use-ticking-now' @@ -18,6 +15,8 @@ import {DetailHeader} from './task-detail-header' import {ErrorSection, InputSection, LiveStreamSection, NotFound, ResultSection} from './task-detail-sections' interface TaskDetailViewProps { + cancelling: boolean + onCancel: (taskId: string) => void taskId: string } @@ -29,7 +28,7 @@ function hasRichDetail(task: StoredTask | undefined): boolean { } // eslint-disable-next-line complexity -export function TaskDetailView({taskId}: TaskDetailViewProps) { +export function TaskDetailView({cancelling, onCancel, taskId}: TaskDetailViewProps) { const storeTask = useTaskById(taskId) const isLiveInStore = storeTask !== undefined && isActiveStatus(storeTask.status) const needsFetch = !hasRichDetail(storeTask) && !isLiveInStore @@ -44,14 +43,6 @@ export function TaskDetailView({taskId}: TaskDetailViewProps) { const tourTaskId = useOnboardingStore((s) => s.tourTaskId) const isTourTask = tourTaskId === taskId - const cancelMutation = useCancelTask() - const handleCancel = (id: string) => { - cancelMutation.mutate( - {taskId: id}, - {onError: (err) => toast.error(err.message)}, - ) - } - const lastReasoning = task?.reasoningContents?.at(-1) const {onScroll, ref: scrollRef} = useStickToBottom>( [ @@ -91,7 +82,7 @@ export function TaskDetailView({taskId}: TaskDetailViewProps) { return (
- +
diff --git a/src/webui/features/tasks/components/task-list-table.tsx b/src/webui/features/tasks/components/task-list-table.tsx index 03d7e8f60..92991c0b6 100644 --- a/src/webui/features/tasks/components/task-list-table.tsx +++ b/src/webui/features/tasks/components/task-list-table.tsx @@ -10,7 +10,7 @@ import { } from '@campfirein/byterover-packages/components/table' import {Tooltip, TooltipContent, TooltipTrigger} from '@campfirein/byterover-packages/components/tooltip' import {cn} from '@campfirein/byterover-packages/lib/utils' -import {CircleStop, Trash2} from 'lucide-react' +import {CircleStop, LoaderCircle, Trash2} from 'lucide-react' import type {StatusFilter} from '../stores/task-store' import type {StoredTask} from '../types/stored-task' @@ -213,7 +213,7 @@ function TaskRow({ {actionKind === 'delete' ? ( onDelete(task.taskId)} /> ) : ( - onCancel(task.taskId)} /> + onCancel(task.taskId)} /> )} @@ -255,17 +255,17 @@ function DeleteRowAction({onClick}: {onClick: () => void}) { ) } -function CancelRowAction({disabled, onClick}: {disabled: boolean; onClick: () => void}) { +function CancelRowAction({cancelling, onClick}: {cancelling: boolean; onClick: () => void}) { return ( ) } diff --git a/src/webui/features/tasks/components/task-list-view.tsx b/src/webui/features/tasks/components/task-list-view.tsx index b10b5f782..99434d8a8 100644 --- a/src/webui/features/tasks/components/task-list-view.tsx +++ b/src/webui/features/tasks/components/task-list-view.tsx @@ -430,7 +430,13 @@ export function TaskListView() { className="data-[side=right]:w-full data-[side=right]:max-w-3xl p-0 shadow-[inset_1px_0_0_rgba(96,165,250,0.18)]" side="right" > - {selectedTaskId && } + {selectedTaskId && ( + + )} From cc333d85f42dac63c6cbcf6ad34bd5e036faca6f Mon Sep 17 00:00:00 2001 From: cuongdo-byterover Date: Sat, 23 May 2026 15:49:17 +0700 Subject: [PATCH 21/21] feat: [ENG-2906] surface ctrl+q in TUI footer, fix selector FIFO, suppress stray 'q' (#697) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Footer hint reads `ctrl+q cancel task`, placed between `↑↓ navigate` and `ctrl+c quit`, mirroring the keybind via selectCancelTargetTaskId so it appears exactly when ctrl+q is armed. - Selector now prefers the OLDEST running task over any queued task, with FIFO fallback to the oldest queued. Fixes the multi-curate bug where ctrl+q cancelled the most-recently submitted task instead of the one occupying the agent slot. - Mirror the existing ctrl+o suppression for ctrl+q in command-input so the chord no longer types a stray `q` into the prompt and subsequent ctrl+q presses still register cleanly. --- src/tui/components/command-input.tsx | 23 ++++++++++--- src/tui/components/footer.tsx | 34 +++++++++++++++---- .../tasks/hooks/select-cancel-target.ts | 32 ++++++++++++----- .../hooks/use-cancel-running-task-keybind.ts | 7 ++-- .../tasks/hooks/select-cancel-target.test.ts | 31 +++++++++++++++-- 5 files changed, 103 insertions(+), 24 deletions(-) diff --git a/src/tui/components/command-input.tsx b/src/tui/components/command-input.tsx index 380f4f8a6..b453e5a77 100644 --- a/src/tui/components/command-input.tsx +++ b/src/tui/components/command-input.tsx @@ -32,6 +32,7 @@ export const CommandInput = () => { const [inputKey, setInputKey] = useState(0) const [activeDialog, setActiveDialog] = useState(null) const ctrlOPressedRef = useRef(false) + const ctrlQPressedRef = useRef(false) const previousInputRef = useRef('') // Placeholder based on onboarding step @@ -40,23 +41,37 @@ export const CommandInput = () => { return 'Type a command...' }, [isStreaming]) - // Filter out "o" character when Ctrl+O is pressed + // Filter out the chord-suffix character when Ctrl+O or Ctrl+Q is pressed: + // ink-text-input receives the literal "o"/"q" alongside the modifier and + // appends it before our useInput handler runs, so we strip it here. useEffect(() => { if (ctrlOPressedRef.current) { - // Check if "o" was just added to the end if (inputValue === previousInputRef.current + 'o') { setInputValue(previousInputRef.current) } ctrlOPressedRef.current = false } + + if (ctrlQPressedRef.current) { + if (inputValue === previousInputRef.current + 'q') { + setInputValue(previousInputRef.current) + } + + ctrlQPressedRef.current = false + } }, [inputValue]) - // Detect Ctrl+O to prevent "o" from being inserted + // Detect Ctrl+O / Ctrl+Q to prevent their letter from being inserted. + // Ctrl+Q is the cancel-task chord; see useCancelRunningTaskKeybind. useInput((input, key) => { - if (key.ctrl && input === 'o') { + if (!key.ctrl) return + if (input === 'o') { previousInputRef.current = inputValue ctrlOPressedRef.current = true + } else if (input === 'q') { + previousInputRef.current = inputValue + ctrlQPressedRef.current = true } }) diff --git a/src/tui/components/footer.tsx b/src/tui/components/footer.tsx index a5996a42f..5764ef69c 100644 --- a/src/tui/components/footer.tsx +++ b/src/tui/components/footer.tsx @@ -6,6 +6,7 @@ import {Box, Spacer, Text} from 'ink' import React from 'react' import {useAppViewMode} from '../features/onboarding/hooks/use-app-view-mode.js' +import {selectCancelTargetTaskId} from '../features/tasks/hooks/select-cancel-target.js' import {useTasksStore} from '../features/tasks/stores/tasks-store.js' import {useMode, useTheme} from '../hooks/index.js' @@ -16,6 +17,9 @@ export const Footer: React.FC = () => { theme: {colors}, } = useTheme() const taskStats = useTasksStore((s) => s.stats) + // Mirrors the keybind in useCancelRunningTaskKeybind so the hint is visible + // exactly when ctrl+q is armed — never advertised when nothing is cancellable. + const hasCancellableTask = useTasksStore((s) => selectCancelTargetTaskId(s.tasks) !== undefined) if (viewMode.type === 'loading' || viewMode.type === 'config-provider') { return @@ -30,13 +34,29 @@ export const Footer: React.FC = () => { {taskStats?.started ?? 0} - {shortcuts.map((shortcut, index) => ( - - {index > 0 && • } - {shortcut.key} - {shortcut.description} - - ))} + {shortcuts.map((shortcut, index) => { + // Inject the cancel-task hint after the first shortcut (navigate) + // so the order reads `↑↓ navigate · ctrl+q cancel task · ctrl+c quit`. + // The hint piggybacks on `selectCancelTargetTaskId`, so it appears + // exactly while the keybind is armed and never advertises a no-op. + const showCancelHintBefore = hasCancellableTask && index === 1 + return ( + + {showCancelHintBefore && ( + + • + ctrl+q + cancel task + + )} + + {index > 0 && • } + {shortcut.key} + {shortcut.description} + + + ) + })} ) } diff --git a/src/tui/features/tasks/hooks/select-cancel-target.ts b/src/tui/features/tasks/hooks/select-cancel-target.ts index 0c1ac8523..d947e5a58 100644 --- a/src/tui/features/tasks/hooks/select-cancel-target.ts +++ b/src/tui/features/tasks/hooks/select-cancel-target.ts @@ -4,19 +4,35 @@ import type {Task, TaskStatus} from '../stores/tasks-store.js' const TERMINAL_STATUSES: ReadonlySet = new Set(['cancelled', 'completed', 'error']) /** - * Pick the taskId Ctrl+Q should target. Returns the most recently created - * non-terminal task in the tasks map, or undefined when there is nothing - * cancellable. Pure function — extracted from the React hook so it can be - * unit-tested without Ink. + * Pick the taskId Ctrl+Q should target. + * + * Policy: prefer the currently-running task over any queued task, and within + * each group prefer the OLDEST. Rationale: when a user has several curate + * tasks in flight and presses Ctrl+Q, the natural expectation is "stop what + * is happening now" — which is the running task occupying the agent slot. + * Cancelling it frees the slot so the next queued task starts immediately. + * The previous "most recent non-terminal" policy violated this intuition + * by silently cancelling whichever task was submitted last instead of the + * one the user perceived as active. + * + * Returns undefined when nothing is cancellable. Pure function — extracted + * from the React hook so it can be unit-tested without Ink. */ export function selectCancelTargetTaskId(tasks: ReadonlyMap): string | undefined { - let candidate: Task | undefined + // Two-pass: first scan for a `started` task (running), then fall back to + // any non-terminal task (covers `created`/queued). + let runningCandidate: Task | undefined + let queuedCandidate: Task | undefined for (const task of tasks.values()) { if (TERMINAL_STATUSES.has(task.status)) continue - if (!candidate || task.createdAt > candidate.createdAt) { - candidate = task + if (task.status === 'started') { + if (!runningCandidate || task.createdAt < runningCandidate.createdAt) { + runningCandidate = task + } + } else if (!queuedCandidate || task.createdAt < queuedCandidate.createdAt) { + queuedCandidate = task } } - return candidate?.taskId + return (runningCandidate ?? queuedCandidate)?.taskId } diff --git a/src/tui/features/tasks/hooks/use-cancel-running-task-keybind.ts b/src/tui/features/tasks/hooks/use-cancel-running-task-keybind.ts index c146d1e43..fdbbd6e7a 100644 --- a/src/tui/features/tasks/hooks/use-cancel-running-task-keybind.ts +++ b/src/tui/features/tasks/hooks/use-cancel-running-task-keybind.ts @@ -15,9 +15,10 @@ * intercepted everywhere — text inputs, REPL prompt, search box, modals, etc. * - DO NOT bind Ctrl+Q to any other action while a curate/query/dream task can * be in flight; the global cancel will eat the chord first. - * - The binding targets `selectCancelTargetTaskId`'s "most recently created - * non-terminal task." With multiple in-flight tasks, Ctrl+Q always cancels - * the most recent one, not the one currently focused in the UI. + * - The binding targets `selectCancelTargetTaskId`'s "oldest running task, + * falling back to the oldest queued task." With multiple in-flight tasks, + * Ctrl+Q cancels the task occupying the agent slot (FIFO) so the queue + * drains; it does NOT cancel whichever task is currently focused in the UI. * - There is no confirmation step today — the cancel network round-trip starts * the instant the chord lands. If a real-world conflict surfaces (user * accidentally cancelling), the recommended follow-up is to mirror the SIGINT diff --git a/test/unit/tui/features/tasks/hooks/select-cancel-target.test.ts b/test/unit/tui/features/tasks/hooks/select-cancel-target.test.ts index 886e077f6..41c8795d0 100644 --- a/test/unit/tui/features/tasks/hooks/select-cancel-target.test.ts +++ b/test/unit/tui/features/tasks/hooks/select-cancel-target.test.ts @@ -37,13 +37,40 @@ describe('selectCancelTargetTaskId', () => { expect(selectCancelTargetTaskId(tasks)).to.equal('b') }) - it('returns the most recently created non-terminal task when several are running', () => { + it('returns the OLDEST running task when several are concurrently running', () => { + // Policy: Ctrl+Q stops what is currently happening. Among running tasks, + // pick the oldest — it has occupied the agent slot longest and is the + // most natural "active" task in the user's mental model. const tasks = new Map([ ['mid', makeTask({createdAt: 200, status: 'started', taskId: 'mid', type: 'query'})], ['new', makeTask({createdAt: 300, status: 'started', taskId: 'new', type: 'curate'})], ['old', makeTask({createdAt: 100, status: 'started', taskId: 'old', type: 'curate'})], ]) - expect(selectCancelTargetTaskId(tasks)).to.equal('new') + expect(selectCancelTargetTaskId(tasks)).to.equal('old') + }) + + it('prefers a running task over any queued (created-status) task', () => { + // Regression for the multi-curate bug: ctrl+q must stop the running task, + // not the most-recently-queued one. Cancelling the runner also frees the + // slot so the queue drains naturally. + const tasks = new Map([ + ['queued-newer', makeTask({createdAt: 300, status: 'created', taskId: 'queued-newer', type: 'curate'})], + ['queued-older', makeTask({createdAt: 150, status: 'created', taskId: 'queued-older', type: 'curate'})], + ['running', makeTask({createdAt: 100, status: 'started', taskId: 'running', type: 'curate'})], + ]) + expect(selectCancelTargetTaskId(tasks)).to.equal('running') + }) + + it('falls back to the OLDEST queued task when nothing is running yet', () => { + // Cold-start scenario: the agent has not yet picked anything up. The + // oldest queued task is closest to dispatch, so cancelling it is the + // FIFO-consistent choice (matches what the user just submitted first). + const tasks = new Map([ + ['a', makeTask({createdAt: 100, status: 'created', taskId: 'a', type: 'curate'})], + ['b', makeTask({createdAt: 200, status: 'created', taskId: 'b', type: 'curate'})], + ['c', makeTask({createdAt: 300, status: 'created', taskId: 'c', type: 'curate'})], + ]) + expect(selectCancelTargetTaskId(tasks)).to.equal('a') }) it('treats `created` status as non-terminal (still cancellable before task:started)', () => {