Skip to content

feat: [ENG-2784] add WebUI cancel button to task list rows and detail…#693

Merged
ncnthien merged 2 commits into
proj/task-cancellationfrom
feat/ENG-2784
May 22, 2026
Merged

feat: [ENG-2784] add WebUI cancel button to task list rows and detail…#693
ncnthien merged 2 commits into
proj/task-cancellationfrom
feat/ENG-2784

Conversation

@ncnthien
Copy link
Copy Markdown
Collaborator

… header

Summary

  • Problem:
  • Why it matters:
  • What changed:
  • What did NOT change (scope boundary):

Type of change

  • Bug fix
  • New feature
  • Refactor (no behavior change)
  • Documentation
  • Test
  • Chore (build, dependencies, CI)

Scope (select all touched areas)

  • TUI / REPL
  • Agent / Tools
  • LLM Providers
  • Server / Daemon
  • Shared (constants, types, transport events)
  • CLI Commands (oclif)
  • Hub / Connectors
  • Cloud Sync
  • CI/CD / Infra

Linked issues

  • Closes #
  • Related #

Root cause (bug fixes only, otherwise write N/A)

  • Root cause:
  • Why this was not caught earlier:

Test plan

  • Coverage added:
    • Unit test
    • Integration test
    • Manual verification only
  • Test file(s):
  • Key scenario(s) covered:

User-visible changes

List user-visible changes (including defaults, config, or CLI output).
If none, write None.

Evidence

Attach at least one:

  • Failing test/log before + passing after
  • Trace/log snippets
  • Screenshot/recording

Checklist

  • Tests added or updated and passing (npm test)
  • Lint passes (npm run lint)
  • Type check passes (npm run typecheck)
  • Build succeeds (npm run build)
  • Commits follow Conventional Commits format
  • Documentation updated (if applicable)
  • No breaking changes (or clearly documented above)
  • Branch is up to date with main

Risks and mitigations

List real risks for this PR. If none, write None.

  • Risk:
    • Mitigation:

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 22, 2026

Claude finished @ncnthien's task in 3m 22s —— View job


Claude PR Review

Score: 4 / 5 — clean, focused implementation with solid unit coverage on the new module boundaries; one real UX consistency bug worth fixing before merge.

Todo:

  • Load review tools
  • Read changed files (8) + adjacent transport / status helpers
  • Check transport / cancel handler integration (TaskEvents.CANCEL, TaskCancelResponse semantics in shared/transport/events/task-events.ts)
  • Audit unit test coverage (cancel-task.test.ts, row-action-kind.test.ts)
  • Post inline comments + summary

Summary

This PR wires up two new entry points to the existing task:cancel daemon flow:

  • A Cancel button in DetailHeader (visible while isActiveStatus).
  • A per-row icon button in TaskTable that swaps between Cancel (non-terminal) and Delete (terminal), driven by a new rowActionKind util.

It also adds the cancelTask / useCancelTask API wrapper and cancellingIds local state in TaskListView to disable the row button during the round-trip.

The scope is right-sized, follows the same pattern as the existing delete/clear-completed mutations, and the import boundary is respected (no server/ or agent/ imports). Tests cover the new pure pieces — but skip the new UI integration, which is consistent with the repo's existing webui test posture.


Findings (inline)

# File Severity Topic
1 task-detail-view.tsx:47-53 issue Detail view spins up its own useCancelTask mutation, so the row-level cancellingIds set and the detail header's cancelMutation.isPending are independent — cancelling from one surface doesn't reflect in the other. See inline.
2 task-list-view.tsx:245-270 nitpick cancellingIds only clears via mutation callbacks; if the terminal task:cancelled event arrives but the reply is dropped, the row stays disabled. Add a status-watching effect as belt-and-suspenders.
3 task-detail-header.tsx:57-69 suggestion While cancelling is true the only feedback is disabled — no Cancelling… label or spinner. Same applies to CancelRowAction in the table.
4 task-list-table.tsx:212-218 question Interrupted tasks (daemon restart) still render the Cancel action even though the row is at opacity-60. Intentional, or should they fall through to Delete?
5 row-action-kind.ts:7-9 praise Clean pure util, fully covered (all 5 TaskListItemStatus values), and gives the call site a self-documenting name vs. an inline ternary.

The blocker-ish one is #1 — the rest are polish.


Correctness / Security / Quality notes

  • Transport contract: cancelTask correctly emits TaskEvents.CANCEL with the documented TaskCancelRequest shape and treats success: false as an error, including a sane fallback message — matches the response semantics documented at shared/transport/events/task-events.ts:56-75.
  • No as Type / any in the new code; types are sourced from shared/transport/events/task-events. ✅
  • Mutation hygiene: handleCancel in task-list-view.tsx uses functional setCancellingIds updates with proper add/remove flows; the clear() closure captures taskId cleanly. ✅
  • Accessibility: Both buttons have aria-label="Cancel task"; row button also has a title. ✅
  • No secrets / no PII / no new network surface — all routed through the existing daemon socket. ✅
  • Stack/lint compliance: ES-module .js imports, no new interface I… prefix needed (only types), and RowActionKind follows the type for data-shape convention from CLAUDE.md. ✅
  • Outside-In: Consumer (table + detail header) drives the new API and util — entities/utils were not designed in isolation. ✅
    · branch feat/ENG-2784

Comment thread src/webui/features/tasks/components/task-detail-view.tsx Outdated
Comment thread src/webui/features/tasks/components/task-list-view.tsx
Comment thread src/webui/features/tasks/components/task-detail-header.tsx
Comment thread src/webui/features/tasks/components/task-list-table.tsx
Comment thread src/webui/features/tasks/utils/row-action-kind.ts
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 22, 2026

Claude finished @ncnthien's task in 1m 19s —— View job


Claude PR Review — re-review after 98c0eb3

Score: 5 / 5 — follow-up commit cleanly addresses both actionable items from the previous review. Ready to merge.

Todo:

  • Diff new commit 98c0eb3 against prior review state
  • Verify issue Feat/init #1 fix: shared cancel state across list + detail
  • Verify issue add the app cli #3 fix: pending feedback (label/spinner) on both surfaces
  • Re-check tests + types
  • Post updated summary

What changed

The follow-up commit 98c0eb3 makes TaskDetailView presentational for cancellation (option 2 from the previous review) and adds the missing in-flight affordance.

  • task-detail-view.tsx:17-31 — drops useCancelTask + local handleCancel; now accepts cancelling: boolean + onCancel: (taskId: string) => void props. Dead toast / useCancelTask imports removed cleanly.
  • task-list-view.tsx:434 — wires cancelling={cancellingIds.has(selectedTaskId)} and onCancel={handleCancel} into the Sheet'd detail. Row and header now read from the same cancellingIds set.
  • task-detail-header.tsx:66-67 — swaps CircleStop for spinning LoaderCircle and label for Cancelling… while pending.
  • task-list-table.tsx:258-270CancelRowAction prop renamed disabledcancelling, icon swaps to spinner, title switches to Cancelling….

Verification

  • Issue Feat/init #1 (shared state): ✅ Cancelling from a row now disables the header for that same task, and vice versa — both surfaces read cancellingIds.has(taskId).
  • Issue add the app cli #3 (pending feedback): ✅ Both buttons show a Loader2/LoaderCircle spinner + Cancelling… text/title while in-flight.
  • Issue Feat/refactor structure #2 (terminal-status leak): defended by author — accepted. Row swaps to DeleteRowAction once the task hits a terminal status (rowActionKind returns 'delete'), so a leaked id has no user-visible effect. Reasonable defense.
  • Issue feat: update readme and contribuing guide #4 (interrupted task → still shows Cancel): not addressed in this commit. Product question, fine to defer.
  • Tests: no new failures expected — TaskDetailView had no dedicated test file, and the existing cancel-task.test.ts / row-action-kind.test.ts are unaffected by the prop-shape change.

Remaining nits (non-blocking, polish only)

File Note
task-detail-header.tsx:59 While cancelling, visible label is Cancelling… but aria-label="Cancel task" stays put. Screen readers will announce "Cancel task, button, dimmed" — consider toggling aria-label to Cancelling task to match the visible text.
task-list-table.tsx:261 Same as above for the icon-only row button — aria-label could flip to Cancelling task while pending.
task-list-table.tsx:268 size-3.5 spinner inherits text color; on a variant="ghost" disabled button the muted color is fine, just confirm it's visible enough at the standard opacity against the row's hover/zebra background.

None are blocking. Nice tight follow-up.
· branch feat/ENG-2784

@ncnthien ncnthien merged commit 4916a50 into proj/task-cancellation May 22, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants