SIGN IN SIGN UP

Proj/task cancellation (#680)

* feat: [ENG-2776] expose taskId-scoped cancel on agent and session

- ChatSession.cancel(taskId?) returns boolean so callers can tell
  whether a controller was found and aborted vs no-op.
- CipherAgent.cancelTask(taskId) fans cancel across live sessions and
  returns true when any session held the controller. Idempotent.
- Legacy no-arg cancel semantics unchanged (REPL Esc path).

* feat: [ENG-2775] agent process subscribes to task:cancel

- New handleAgentCancelEvent helper delegates to CipherAgent.cancelTask
  and emits task:cancelled upstream only when an active controller was
  held. Best-effort: any error from agent or transport is logged and
  swallowed so the cancel pipeline never crashes the agent.
- agent-process registers the listener alongside task:execute.
- Unit tests cover matched emit, unmatched no-op, agent-throw and
  transport-throw paths, idempotency, and per-event logging.

* feat: [ENG-2777] cancel-aware execute, queue advance, queued-task cancel

- handleExecutorTerminalError helper branches the executor catch:
  SessionCancelledError suppresses task:error (T1.1 emits task:cancelled);
  anything else takes the existing task:error path. Curate Phase 4 is
  naturally skipped because the executor throws before postWork is
  assigned; dream's finally still releases the lock via rollback on the
  abort path.
- task-router.handleTaskCancelled now calls agentPool.notifyTaskCompleted
  so the project FIFO drains after a cancel, symmetric with the
  completed/error paths.
- New IAgentPool.cancelQueuedTask delegates to ProjectTaskQueue.cancel.
  task-router.handleTaskCancel checks the queue first; queued tasks are
  removed and the daemon emits task:cancelled directly, never forwarding
  to the agent that holds no controller for them.
- Extract cancelTaskLocally to share broadcast and hook plumbing between
  the queued-cancel and no-agent-connected branches.
- Tests: 5 for handleExecutorTerminalError, 4 for task-router cancel
  paths, 3 for agent-pool cancelQueuedTask. Update 7 existing mock
  IAgentPool to satisfy the new interface method.

* feat: [ENG-2778] integration test for end-to-end cancel pipeline

- New test/integration/cancel-pipeline.test.ts covers the 5 T1.4
  scenarios: cancel running, cancel running with queued, cancel queued,
  idempotent double-cancel, follow-up after cancel.
- Harness uses a real SocketIOTransportServer, a real TaskRouter, and a
  stub IAgentPool backed by a real ProjectTaskQueue so the queue/drain
  logic is exercised end-to-end. A TransportClient acts as the mock
  agent and honours per-task behaviors (wait-for-cancel or auto-complete)
  so the test driver controls when each task ends.
- History persistence is asserted via an ITaskLifecycleHook recorder
  rather than a real FileCurateLogStore — the recorder is the same hook
  interface CurateLogHandler implements in production to write
  status='cancelled' to history. The file-write itself stays covered by
  file-curate-log-store unit tests.
- Suite stays in-memory; waitFor polls with a bounded 1500ms deadline.
  Whole file finishes in ~210ms with no hanging promises.

* feat: [ENG-2779] shared CLI helper that emits the cancel request

- New src/oclif/lib/cancel-task.ts exposes runCancelTask({client,
  command, format, log, taskId}) — the only place in the CLI where the
  task:cancel transport event name and request shape appear. T2.2-T2.5
  per-command --cancel flags will call it.
- Text format prints "Cancelled <id>" on success, "Failed to cancel
  <id>: <reason>" on daemon-reported failure, with a generic fallback
  when the daemon omits the reason.
- JSON format reuses writeJsonResponse so the payload shape stays
  consistent with other commands' --format json output. The caller's
  command name is stamped on the envelope so curate/query/dream stay
  greppable.
- Returns boolean so the per-command caller can decide on exit code.
- 7 unit tests cover both formats, success and failure paths, transport
  payload shape, and the missing-error-reason fallback.

* feat: [ENG-2780] brv curate --cancel flag

- New --cancel <id> flag short-circuits the create flow: no provider
  validation, no task:create, no context required. Delegates to the
  shared runCancelTask helper from T2.1 and exits 1 on daemon-reported
  failure so scripts can rely on the exit code.
- Mutually exclusive with --files, --folder, --detach (oclif's
  `exclusive`). --timeout is left allowed and silently ignored on the
  cancel branch; rejecting a harmless combo would surprise users.
- Existing curate behavior is untouched. The cancel branch is added
  alongside the normal path with zero overlap on the create code.
- 10 new tests in test/commands/curate.test.ts cover both formats,
  success and failure paths, mutex enforcement against each of the
  three exclusive flags, and the timeout-allowed case.

* feat: [ENG-2781] brv query --cancel flag

- Add --cancel <id> flag that short-circuits the query flow through
  the shared runCancelTask helper from T2.1.
- Relax the positional `query` argument to optional so --cancel can be
  used without a placeholder query string. A manual guard in run()
  preserves the existing missing-query error when neither input is
  given.
- Reject "<query> --cancel <id>" as a mutually exclusive combination
  with a clear error message in both text and JSON formats, exiting
  non-zero without touching the transport.
- Help text on both surfaces reflects the dual mode (run a query, or
  cancel by id).
- 7 new tests cover both formats, success and failure paths, the
  positional+cancel conflict, and missing-both rejection.

* feat: [ENG-2782] brv dream --cancel flag

- Add --cancel <id> flag that short-circuits the dream flow through
  the shared runCancelTask helper from T2.1. Branches BEFORE the
  --undo path so cancel never accidentally triggers a revert.
- Mutually exclusive with --force, --undo, --detach (oclif's
  `exclusive`). --timeout is left allowed and silently ignored on the
  cancel branch.
- Help text makes clear that cancel is a hard stop and not a revert;
  pair with `brv dream --undo` for rollback of completed/partial
  dreams.
- 9 new tests cover both formats, success and failure paths, the
  mutex against each of the three exclusive flags, and the
  timeout-allowed case.

* feat: [ENG-2783] foreground Ctrl-C sends cancel before exit

- waitForTaskCompletion installs a SIGINT handler for the duration of
  the wait. First Ctrl-C emits task:cancel for the current task and
  writes a "Cancelling task..." hint to stderr (kept off stdout so
  --format json output stays clean). Second Ctrl-C hard-exits with
  code 130 (conventional SIGINT exit) without re-emitting cancel.
- The wait loop now subscribes to task:cancelled as a terminal event.
  Previously it only handled task:completed and task:error, which
  meant a remote --cancel or a Ctrl-C cancel left the foreground
  process hanging until the existing timeout fired. The new branch
  fires the optional onCancelled callback and resolves the promise.
- The SIGINT handler is added to the same unsubscribers chain the
  existing cleanup() drains, so it is removed on every terminal path
  (completed, cancelled, error, disconnect, timeout).
- 8 new unit tests in test/unit/oclif/lib/task-client-sigint.test.ts
  cover both terminal recognition and the SIGINT install/uninstall
  contract.

* feat: [ENG-2787] TUI cancel-task API helper

- Add src/tui/features/tasks/api/cancel-task.ts mirroring the
  provider/api/cancel-oauth pattern: pulls the active apiClient from
  the TUI transport store, emits task:cancel with the taskId, returns
  the daemon's TaskCancelResponse, throws when the response reports
  success: false.
- No new transport client construction; reuses the established TUI
  pattern. Boundary preserved — no imports from server/, agent/, or
  oclif/.
- 5 unit tests cover the event payload, the success resolve, the
  daemon-reported error path, the missing-error fallback, and the
  not-connected guard. Test file establishes the new
  test/unit/tui/features/tasks/api/ folder for T4.2 to extend.

* feat: [ENG-2788] Ctrl+Q cancel keybind for active curate/query tasks

- New useCancelRunningTaskKeybind hook installs a scoped useInput
  that fires only while there is a non-terminal curate/query task in
  the tasks store. On Ctrl+Q the hook calls cancelTask({taskId}) for
  the most recently created running task.
- Architectural note: curate-flow and query-flow components unmount
  within ~100ms of task:create ack, so they cannot own a binding that
  must be active for the entire task lifetime. The keybind is scoped
  by running-task presence in the store, hosted via an invisible
  CancelKeybindInitializer mounted next to TaskSubscriptionInitializer.
  REPL UX is unchanged (prompt stays responsive while task runs).
- A pure selectCancelTargetTaskId selector picks the target taskId —
  most recently created non-terminal task, or undefined when nothing
  is cancellable. Splitting the pure logic from the React hook keeps
  it unit-testable without Ink.
- Ctrl-C remains the TUI's exit shortcut; only Ctrl+Q is intercepted.
  Known limitation: some terminal emulators map Ctrl+Q to XON flow-
  control; Ink raw mode should neutralize that on common emulators
  but is not guaranteed everywhere.
- 6 unit tests cover the selector: empty map, all-terminal, single
  running, multiple running (newest wins), created-status, and
  terminal-newer-than-running.

* refactor: [ENG-2783] DRY cancel-branch wiring + tighten SIGINT and query typing

Post-review cleanup of T2.5 (ENG-2783):

- Extract runCancelBranchWithRetry into src/oclif/lib/cancel-task.ts so
  curate / query / dream stop cloning the same 22-line withDaemonRetry
  wrapper. Per-command runCancelBranch becomes a 10-line delegation:
  pass command name, daemon-client options, format, log, transport-error
  handler, and taskId; receive a boolean for exit-code decision.
- Wrap the SIGINT handler's client.request call in try/catch so a
  synchronously-throwing transport (already-disconnected socket, etc.)
  cannot escape the signal handler and crash the process with an
  uncaught exception. The wait loop still surfaces the disconnect via
  timeout or state-change paths.
- Drop the redundant 'as QueryFlags' cast in query.ts entirely along
  with the now-unused QueryFlags type alias. Use oclif's inferred flag
  type directly and narrow 'format' inline via a ternary, matching the
  pattern in curate/index.ts.
- 3 new unit tests for runCancelBranchWithRetry: success path, daemon-
  reported failure (no transport throw), and transport-throw routed to
  onTransportError. Existing per-command --cancel tests pass unchanged
  because the per-command behaviour is the same.

* feat: [ENG-2783] foreground commands surface remote --cancel

When the foreground curate / query / dream wait loop sees a
task:cancelled broadcast (driven by a remote 'brv <cmd> --cancel <id>'
from another terminal, or the TUI Ctrl+Q keybind), the command now:

- prints '✗ <Command> cancelled (Task: <id>)' in text mode, or writes
  a {status: 'cancelled', event: 'cancelled'} JSON envelope in json
  mode, so consumers can distinguish cancel from success;
- exits with code 130 (the conventional SIGINT exit) so shell scripts
  and CI can branch on cancellation.

Wiring detail worth noting: the wasCancelled flag is hoisted to run()
and bubbled up by submitTask via its return shape. The this.exit(130)
call lives AFTER the withDaemonRetry try/catch — throwing it inside
the callback would be swallowed by the catch (which routes any throw
through reportError) and the exit code would be lost.

6 new unit tests cover the remote-cancel path in text and json mode
across all three commands. Existing tests still pass unchanged.

* fix: [ENG-2783] daemon-side idempotency for task:cancel retries

When withDaemonRetry retries a task:cancel after a dropped response, the
daemon's handleTaskCancel would look up the taskId in this.tasks, miss
(the prior cancel had already moved it to completedTasks), and return
{error: 'Task not found', success: false}. The CLI then reported a
misleading 'Failed to cancel' even though the cancel actually worked.

Make the lookup idempotent: if the taskId is absent from this.tasks but
present in completedTasks with status: 'cancelled', return success.
Truly unknown taskIds and tasks that reached a different terminal state
(completed / error) still return the structured error, so the existing
contract for those cases is preserved.

This covers Nit-3 from the cancel-pipeline post-review.

1 new unit test in test/unit/infra/process/task-router.test.ts asserts
the retry-after-cancel path returns {success: true}. Existing tests
including 'should return error for unknown taskId' continue to pass.

* chore: [ENG-2783] log swallowed SIGINT errors + align cancel JSON success flag

Two small post-review nits from the cancel-pipeline review:

Nit-4 — empty catch {} in the SIGINT handler hid programmer-error
  throws (bad payload, transport regression) silently. Add a single
  stderr write inside the catch so a genuine bug leaves a breadcrumb.
  Comment expanded to spell out why we still swallow rather than
  propagate (the wait loop's timeout / state-change paths already
  surface a true transport drop).

Nit-5 — the cancelled-path JSON envelope previously set success: true
  while the process exits 130. Two signals pointing opposite ways
  confused consumers parsing both. Flip JSON success to false so it
  tracks the exit code; cancellation semantics still live in
  data.status: 'cancelled' for code that needs to distinguish cancel
  from error. Updated the 3 'remote cancel during foreground wait' JSON
  tests across curate/query/dream to expect success: false. No
  behaviour change in text mode or in exit codes.

Touches src/oclif/lib/task-client.ts and the 3 CLI command files only;
test/commands/{curate,query,dream}.test.ts get the matching assertion
flip.

* fix: [ENG-2783] durable cancel idempotency via persistent history store

The previous Nit-3 fix only looked up the cancelled status in the
in-memory `completedTasks` map. That map is cleared by a setTimeout
after `TASK_CLEANUP_GRACE_PERIOD_MS` (5s), so a cancel retry arriving
after the grace window would still report "Task not found" even though
the task was successfully cancelled — semantically wrong, since the
task is in fact cancelled in the persistent history.

Extend handleTaskCancel with a durable second fallback: when the task
is gone from both this.tasks AND completedTasks, look it up by id in
the persistent task history store (resolved via the client's project
path). If the persisted status is 'cancelled', return success. Any
other status (completed / error) or a missing entry still returns the
structured "Task not found".

The handler becomes async because store.getById is async; the existing
sync test invocations get `await` added. No production caller depended
on the sync return — `transport.onRequest` supports both shapes.

2 new tests in task-router.test.ts:
- 'returns success on a retry after completedTasks has aged out —
  durable via persistent history store'
- 'still returns Task not found when the history store has a
  non-cancelled status (e.g. completed)'

The original in-memory fast path is preserved so the common case
(retry within 5s) avoids a disk read.

* fix: [ENG-2783] cancelTaskLocally cache miss + post-review nits

- task-router: cancelTaskLocally now stamps status='cancelled' and routes
  through moveToCompleted so the in-memory fast-path idempotency catches
  sequential retries on the queued/no-agent cancel paths. Previously the
  retry raced the fire-and-forget durable hook write and could see a
  misleading 'Task not found'. Adds regression test.
- transport: document TaskCancelResponse.success semantics across the
  four reply paths (queued, forwarded, idempotent retry, not-found),
  plus an inline note above the forward-to-agent branch in task-router.
- oclif: inline the runCancelBranch wrapper in curate/dream/query — the
  three identical 9-line private methods are gone; the single use site
  in each command now calls runCancelBranchWithRetry directly.
- task-client: introduce WaitForTaskClient = Pick<ITransportClient, ...>
  so the SIGINT test stub satisfies the contract structurally without
  an 'as unknown as ITransportClient' cast; installSigintCancel narrowed
  to Pick<..., 'request'> for the same reason.
- tui: expand JSDoc on use-cancel-running-task-keybind with a RESERVED
  CHORD WARNING so contributors know Ctrl+Q is globally intercepted
  whenever any non-terminal task exists in the tasks store.

* test: [ENG-2783] drop stale timeoutMs from SIGINT test options

The waitForTaskCompletion options interface lost timeoutMs when the
per-call setTimeout was replaced with a heartbeat-driven stale check,
but this test file kept the field, breaking the suite with TS2353.

* feat: [ENG-2784] add WebUI cancel button to task list rows and detail header

* feat: [ENG-2784] unify cancel state across list and detail, add pending feedback

* feat: [ENG-2906] surface ctrl+q in TUI footer, fix selector FIFO, suppress stray 'q' (#697)

- Footer hint reads `ctrl+q cancel task`, placed between `↑↓ navigate`
  and `ctrl+c quit`, mirroring the keybind via selectCancelTargetTaskId
  so it appears exactly when ctrl+q is armed.
- Selector now prefers the OLDEST running task over any queued task, with
  FIFO fallback to the oldest queued. Fixes the multi-curate bug where
  ctrl+q cancelled the most-recently submitted task instead of the one
  occupying the agent slot.
- Mirror the existing ctrl+o suppression for ctrl+q in command-input so
  the chord no longer types a stray `q` into the prompt and subsequent
  ctrl+q presses still register cleanly.

---------

Co-authored-by: ncnthien <nhatthien185@gmail.com>
Co-authored-by: cuongdo-byterover <cuong@byterover.dev>
B
bao-byterover committed
893f28219642a1f95b3d2ef24e282f85c56dbbd6
Parent: d903cb3
Committed by GitHub <noreply@github.com> on 5/25/2026, 3:59:22 AM