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 91e5a42eb..d4e57683a 100644 --- a/src/agent/infra/agent/cipher-agent.ts +++ b/src/agent/infra/agent/cipher-agent.ts @@ -211,6 +211,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/src/oclif/commands/curate/index.ts b/src/oclif/commands/curate/index.ts index 11995ad11..4006e7dda 100644 --- a/src/oclif/commands/curate/index.ts +++ b/src/oclif/commands/curate/index.ts @@ -10,6 +10,7 @@ import {ProviderConfigResponse, TransportStateEventNames} from '../../../server/ import {extractCurateOperations} from '../../../server/utils/curate-result-parser.js' import {TaskEvents} from '../../../shared/transport/events/index.js' import {printBillingLine} from '../../lib/billing-line.js' +import {runCancelBranchWithRetry} from '../../lib/cancel-task.js' import { type DaemonClientOptions, formatConnectionError, @@ -71,6 +72,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', @@ -113,6 +118,19 @@ Bad examples: } const format: 'json' | 'text' = flags.format ?? 'text' + if (rawFlags.cancel) { + 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 + } + warnIfTimeoutFlagUsed({ defaultValue: DEFAULT_TIMEOUT_SECONDS, log: (message) => this.log(message), @@ -129,6 +147,7 @@ Bad examples: const taskType = flags.folder?.length ? 'curate-folder' : 'curate' let providerContext: ProviderErrorContext | undefined + let wasCancelled = false try { await withDaemonRetry( @@ -154,7 +173,16 @@ Bad examples: await ensureBillingFunds({billing, client}) } - 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(), @@ -167,7 +195,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) } /** @@ -309,7 +343,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() @@ -339,11 +373,26 @@ 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') { + // 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: false, + }) + } 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 @@ -397,7 +446,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 b15f242c5..f02796528 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 {runCancelBranchWithRetry} from '../lib/cancel-task.js' import { type DaemonClientOptions, formatConnectionError, @@ -84,6 +85,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', @@ -118,6 +123,19 @@ export default class Dream extends Command { const {flags: rawFlags} = await this.parse(Dream) const format = rawFlags.format === 'json' ? 'json' : 'text' + if (rawFlags.cancel) { + 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 + } + warnIfTimeoutFlagUsed({ defaultValue: DEFAULT_TIMEOUT_SECONDS, log: (message) => this.log(message), @@ -130,6 +148,7 @@ export default class Dream extends Command { } let providerContext: ProviderErrorContext | undefined + let wasCancelled = false try { await withDaemonRetry( @@ -149,7 +168,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, @@ -157,6 +176,7 @@ export default class Dream extends Command { projectRoot, worktreeRoot, }) + if (result.wasCancelled) wasCancelled = true }, { ...this.getDaemonClientOptions(), @@ -169,7 +189,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 { @@ -228,7 +254,7 @@ export default class Dream extends Command { format: 'json' | 'text' projectRoot?: string worktreeRoot?: string - }): Promise { + }): Promise<{wasCancelled: boolean}> { const {client, detach, force, format, projectRoot, worktreeRoot} = props const taskId = randomUUID() const taskPayload = { @@ -255,11 +281,26 @@ 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') { + // 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: false, + }) + } else { + this.log(`✗ Dream cancelled (Task: ${tid})`) + } + }, onCompleted: ({logId, result, taskId: tid}) => { const skipped = result?.startsWith('Dream skipped:') if (format === 'json') { @@ -291,6 +332,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 d8ebcf0d4..d8ec4dca1 100644 --- a/src/oclif/commands/query.ts +++ b/src/oclif/commands/query.ts @@ -6,6 +6,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 {printBillingLine} from '../lib/billing-line.js' +import {runCancelBranchWithRetry} from '../lib/cancel-task.js' import { type DaemonClientOptions, formatConnectionError, @@ -19,17 +20,11 @@ import {writeJsonResponse} from '../lib/json-response.js' import {DEFAULT_TIMEOUT_SECONDS, MAX_TIMEOUT_SECONDS, MIN_TIMEOUT_SECONDS, waitForTaskCompletion} from '../lib/task-client.js' import {TIMEOUT_DEPRECATION_HELP, warnIfTimeoutFlagUsed} from '../lib/timeout-deprecation.js' -/** Parsed flags type */ -type QueryFlags = { - format?: 'json' | 'text' - timeout?: number -} - 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 @@ -49,6 +44,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)', @@ -68,19 +66,38 @@ Bad: } public async run(): Promise { - const {args, flags: rawFlags} = await this.parse(Query) - const flags = rawFlags as QueryFlags - 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() !== '') { + this.reportCombinationError(format) + this.exit(1) + return + } + + 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 + } warnIfTimeoutFlagUsed({ defaultValue: DEFAULT_TIMEOUT_SECONDS, log: (message) => this.log(message), - userValue: rawFlags.timeout as number | undefined, + userValue: flags.timeout, }) - if (!this.validateInput(args.query, format)) return + if (!this.validateInput(args.query ?? '', format)) return let providerContext: ProviderErrorContext | undefined + let wasCancelled = false try { await withDaemonRetry( @@ -106,13 +123,14 @@ Bad: await ensureBillingFunds({billing, client}) } - await this.submitTask({ + const result = await this.submitTask({ client, format, projectRoot, - query: args.query, + query: args.query ?? '', worktreeRoot, }) + if (result.wasCancelled) wasCancelled = true }, { ...this.getDaemonClientOptions(), @@ -125,6 +143,25 @@ 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 { + 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) } } @@ -149,7 +186,7 @@ Bad: projectRoot?: string query: string worktreeRoot?: string - }): Promise { + }): Promise<{wasCancelled: boolean}> { const {client, format, projectRoot, query, worktreeRoot} = props const taskId = randomUUID() const taskPayload = { @@ -162,12 +199,27 @@ Bad: } let finalResult: string | undefined + let wasCancelled = false const completionPromise = waitForTaskCompletion( { client, command: 'query', format, + 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: false, + }) + } else { + this.log(`✗ Query cancelled (Task: ${tid})`) + } + }, onCompleted: ({durationMs, matchedDocs, result, taskId: tid, tier, topScore}) => { const previousResult = finalResult @@ -238,6 +290,7 @@ Bad: ) await client.requestWithAck(TaskEvents.CREATE, taskPayload) await completionPromise + return {wasCancelled} } private validateInput(query: string, format: 'json' | 'text'): boolean { diff --git a/src/oclif/lib/cancel-task.ts b/src/oclif/lib/cancel-task.ts new file mode 100644 index 000000000..4db95dfb4 --- /dev/null +++ b/src/oclif/lib/cancel-task.ts @@ -0,0 +1,100 @@ +/** + * 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 {type DaemonClientOptions, withDaemonRetry} from './daemon-client.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 +} + +export type RunCancelBranchOptions = { + /** Name of the invoking CLI command — stamped on the JSON envelope. */ + command: string + /** Forwarded to withDaemonRetry. Lets each command override retry/connector for tests. */ + daemonClientOptions: DaemonClientOptions + /** Output format. */ + format: 'json' | 'text' + /** Text-mode log sink (typically `(msg) => 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 49c9574be..f82a5a2d0 100644 --- a/src/oclif/lib/task-client.ts +++ b/src/oclif/lib/task-client.ts @@ -82,14 +82,23 @@ 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 */ 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 */ @@ -164,6 +173,50 @@ export function formatToolDisplay(toolName: string, args: Record; taskId: string}): () => 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. 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 (error) { + // Transport unavailable — the wait loop will time out or see a + // 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`) + } + } + + 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(). @@ -173,7 +226,7 @@ export function formatToolDisplay(toolName: string, args: Record void): Promise { - const {client, command, format, onCompleted, onError, onResponse, taskId} = options + const {client, command, format, onCancelled, onCompleted, onError, onResponse, taskId} = options const isText = format === 'text' return new Promise((resolve, reject) => { @@ -390,21 +443,17 @@ export function waitForTaskCompletion(options: WaitForTaskOptions, log: (msg: st } }), - // Task cancelled — terminal state, mirrors the daemon-side stop list - // in M6 T1. Dispose without rejecting so a user-cancelled task closes - // cleanly and no phantom "Daemon is unresponsive" fires afterward. + // Task cancelled (terminal) — broadcast by the daemon 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 and no phantom "Daemon is unresponsive" + // fires afterward. The per-command `onCancelled` callback owns the + // user-facing message (success flag, exit code semantics). client.on<{taskId: string}>(TaskEvents.CANCELLED, (payload) => { if (payload.taskId !== taskId || completed) return completed = true cleanup() - if (!isText) { - writeJsonResponse({ - command, - data: {event: 'cancelled', status: 'cancelled', taskId}, - success: true, - }) - } - + onCancelled?.({taskId, toolCalls}) resolve() }), @@ -437,6 +486,7 @@ export function waitForTaskCompletion(options: WaitForTaskOptions, log: (msg: st () => { if (disconnectTimer) clearTimeout(disconnectTimer) }, + installSigintCancel({client, taskId}), ] const cleanup = (): void => { 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-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-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 6f779b7ea..8c49a7fe6 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 { BillingPinChangedPayload, @@ -69,6 +70,8 @@ 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 {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' @@ -441,6 +444,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}) @@ -691,16 +700,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 0dc92dab7..7f6e9e8e8 100644 --- a/src/server/infra/process/task-router.ts +++ b/src/server/infra/process/task-router.ts @@ -603,6 +603,28 @@ 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, + ) + // 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(() => {}) + } + /** * Drain the dirty set: for each taskId still active, fire `onTaskUpdate` on * each lifecycle hook. Tasks moved to `completedTasks` between markDirty @@ -631,17 +653,67 @@ 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 — 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} } - // If Agent connected for this task's project, forward cancel request + // 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. + // 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}) @@ -650,17 +722,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) this.heartbeatManager?.recordTermination(taskId) return {success: true} @@ -691,6 +753,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/src/shared/transport/events/task-events.ts b/src/shared/transport/events/task-events.ts index eb4c65875..72ba6bd06 100644 --- a/src/shared/transport/events/task-events.ts +++ b/src/shared/transport/events/task-events.ts @@ -53,6 +53,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/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/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/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..d947e5a58 --- /dev/null +++ b/src/tui/features/tasks/hooks/select-cancel-target.ts @@ -0,0 +1,38 @@ +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. + * + * 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 { + // 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 (task.status === 'started') { + if (!runningCandidate || task.createdAt < runningCandidate.createdAt) { + runningCandidate = task + } + } else if (!queuedCandidate || task.createdAt < queuedCandidate.createdAt) { + queuedCandidate = task + } + } + + 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 new file mode 100644 index 000000000..fdbbd6e7a --- /dev/null +++ b/src/tui/features/tasks/hooks/use-cancel-running-task-keybind.ts @@ -0,0 +1,52 @@ +/** + * 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. + * + * 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 "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 + * double-tap pattern in `installSigintCancel` (first Ctrl+Q arms, second + * within Ns fires). + */ + +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/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..360b74a69 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, LoaderCircle} 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..9f21f07b9 100644 --- a/src/webui/features/tasks/components/task-detail-view.tsx +++ b/src/webui/features/tasks/components/task-detail-view.tsx @@ -15,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 } @@ -26,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 @@ -80,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 e1ea597d7..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 {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' @@ -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} @@ -398,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 && ( + + )} 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/commands/curate.test.ts b/test/commands/curate.test.ts index eb1137f1a..a7f903297 100644 --- a/test/commands/curate.test.ts +++ b/test/commands/curate.test.ts @@ -578,4 +578,198 @@ 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']) + }) + }) + + // ==================== 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') + // 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 b357f4bfe..cfefd264e 100644 --- a/test/commands/dream.test.ts +++ b/test/commands/dream.test.ts @@ -158,4 +158,184 @@ 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']) + }) + }) + + // ==================== 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') + // 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 65fde23bb..a9b59b595 100644 --- a/test/commands/query.test.ts +++ b/test/commands/query.test.ts @@ -661,4 +661,164 @@ 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) + }) + }) + + // ==================== 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') + // success: false tracks the non-zero exit code; cancellation semantics live in data.status. + expect(cancelled!.success).to.equal(false) + }) + }) }) 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') + }) +}) 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/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() { 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'}) + }) +}) 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..49ae63dc2 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) @@ -800,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( @@ -813,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}) @@ -833,13 +843,150 @@ 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, 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 + // 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 = 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', async () => { + agentPool.cancelQueuedTask.returns(true) + const handler = transportHelper.requestHandlers.get(TransportTaskEventNames.CANCEL) + + const result = await 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)', async () => { + agentPool.cancelQueuedTask.returns(true) + const handler = transportHelper.requestHandlers.get(TransportTaskEventNames.CANCEL) + + 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)', async () => { + agentPool.cancelQueuedTask.returns(false) + const handler = transportHelper.requestHandlers.get(TransportTaskEventNames.CANCEL) + + const result = await 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 + }) + + 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/infra/process/transport-handlers.test.ts b/test/unit/infra/process/transport-handlers.test.ts index 0020472c8..c370b0c67 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(), @@ -551,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') @@ -559,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( @@ -569,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 @@ -579,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}) @@ -588,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}) }) @@ -741,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') @@ -756,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') @@ -774,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}) }) @@ -1081,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') @@ -1092,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) @@ -1492,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') @@ -1500,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}) } }) 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') + }) +}) 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) + }) +}) 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..d88b6e751 --- /dev/null +++ b/test/unit/oclif/lib/task-client-sigint.test.ts @@ -0,0 +1,298 @@ +import {expect} from 'chai' +import {createSandbox, type SinonSandbox, type SinonStub} from 'sinon' + +import {type WaitForTaskClient, waitForTaskCompletion} from '../../../../src/oclif/lib/task-client.js' + +type EventHandler = (data: unknown) => void + +/** + * 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() + 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 requestStub = sandbox.stub() + + const client: WaitForTaskClient = { + on: onStub, + onStateChange: onStateChangeStub, + request: requestStub, + } + + return { + client, + emit(event: string, data: unknown) { + eventHandlers.get(event)?.(data) + }, + onStub, + requestStub, + 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', + }, + () => {}, + ) + + // 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', + }, + () => {}, + ) + + 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', + }, + () => {}, + ) + + 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', + }, + () => {}, + ) + + 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', + }, + () => {}, + ) + + // 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', + }, + () => {}, + ) + + 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', + }, + () => {}, + ) + + 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', + }, + () => {}, + ) + + 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 + }) +}) 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') + } + }) +}) 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..41c8795d0 --- /dev/null +++ b/test/unit/tui/features/tasks/hooks/select-cancel-target.test.ts @@ -0,0 +1,90 @@ +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 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('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)', () => { + 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') + }) +}) 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') + }) +})