feat(flows): add a Stop button to the Workflow Copilot composer - #5223
Conversation
The main chat's Send button morphs into a Stop button while a turn is in flight (ChatComposer's showStopButton, wired to handleStopGeneration -> chatCancel). The Workflow Copilot already renders ChatComposer but never passed onStopGeneration, so the Stop button never appeared and an in-flight builder turn could not be cancelled. Expose a stop() from useWorkflowBuilderChat that cancels the current thread's turn via the same chatCancel (openhuman.channel_web_cancel) primitive the main chat uses -- the flows_build turn streams over the same web-channel surface, so the thread-scoped cancel applies -- and pass it as onStopGeneration to the copilot's ChatComposer. No-op when nothing is in flight or no thread is bound.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughWorkflow builder generation can now be stopped through ChangesWorkflow build cancellation
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant ChatComposer
participant WorkflowCopilotPanel
participant useWorkflowBuilderChat
participant flows_build_cancel
participant flows_build
ChatComposer->>WorkflowCopilotPanel: onStopGeneration()
WorkflowCopilotPanel->>useWorkflowBuilderChat: stop()
useWorkflowBuilderChat->>flows_build_cancel: cancel(thread_id)
flows_build_cancel->>flows_build: signal registered cancellation token
flows_build-->>useWorkflowBuilderChat: partial result with cancelled state
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
coderabbitai
Bot
added
the
feature
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: df1b66c253
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| void chatCancel(threadId).then(cancelled => { | ||
| log('stop: chatCancel thread=%s ok=%s', threadId, cancelled); | ||
| if (cancelled) setLocalSending(false); |
There was a problem hiding this comment.
When Stop is clicked during any flows_build turn, this reports success without cancelling the builder: flows_build invokes Agent::run_single directly and only attaches a progress bridge, while channel_web_cancel in src/openhuman/web_chat/ops.rs can cancel only a registered web-chat turn or task-dispatcher run. Since neither registry contains this builder, the RPC returns cancelled: false; moreover, chatCancel ignores that payload and resolves true, so this callback hides the Stop button while the agent continues running and may still emit or save a proposal. Register the core-side builder run with a cancellation mechanism and propagate the actual cancellation result before clearing localSending.
AGENTS.md reference: AGENTS.md:L29-L30
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed and fixed — thank you, this was correct. flows_build runs agent.run_single inline and is registered in neither web_chat::IN_FLIGHT nor the task-dispatcher registry, so channel_web_cancel returned cancelled:false while chatCancel reported true. Rather than force flows into the web-chat cancel path, this now has real, domain-scoped cancellation: a new build-turn registry (flows/build_registry.rs, thread_id-keyed CancellationToken, request-scoped stale-cancel guard mirroring task_dispatcher::cancel_session_scoped), flows_build wraps its scoped+timeout run in tokio::select!{ r = run => r, _ = token.cancelled() => Cancelled } (no spawn, so the approval/streaming task-local scopes stay intact), and a new openhuman.flows_build_cancel RPC signals it. The FE stop() now calls flowsBuildCancel and reads the REAL cancelled field (not the unconditional true). A user Stop short-circuits before the trail-off backstop and returns a clean chat_done, so the turn ends without a synthesized "continue?" question. Commits e12c8b7, ed40e0d, 3cc8123, caddbac.
|
| Filename | Overview |
|---|---|
| src/openhuman/flows/build_registry.rs | New process-local registry for in-flight flows_build turns; uses std::sync::Mutex (correct — never held across await), CancellationToken for signalling, and scoped-cancel matching identical to task_dispatcher::cancel_session_scoped. Comprehensive unit tests cover all guard branches. |
| src/openhuman/flows/ops.rs | flows_build: registers a CancellationToken before racing the inline agent run via tokio::select!, then unregisters unconditionally on every exit path (run, timeout, cancel). Cancelled branch calls finalize_flow_stream so the FE receives chat_done and nothing dangles. flows_build_cancel delegates to the registry. |
| app/src/hooks/useWorkflowBuilderChat.ts | Adds stop() (fire-and-forget flowsBuildCancel, no eager localSending clear) and sendAttemptRef to prevent a cancelled turn's finally from wiping a newer send's in-flight flag. All guard conditions (no thread, not sending) are correctly checked. |
| app/src/services/api/flowsApi.ts | Adds flowsBuildCancel wrapping openhuman.flows_build_cancel; correctly maps undefined requestId to null for unscoped cancel, handles missing cancelled field with ?? false. |
| app/src/components/flows/WorkflowCopilotPanel.tsx | Minimal two-line change: destructures stop from the hook and passes it as onStopGeneration to ChatComposer. No logic changes. |
| src/openhuman/flows/schemas.rs | Registers build_cancel schema and handler; thread_id required, request_id optional (matching the cancel's scoped/unscoped semantics). Controller count assertion updated to 34. |
| app/src/hooks/useWorkflowBuilderChat.test.ts | Adds 5 new tests covering: happy-path cancel, no-op when idle, no-op without thread, swallowed rejected RPC, and the re-send race regression. All scenarios are correctly set up with manual promise control. |
| app/src/services/api/flowsApi.test.ts | Adds 4 tests for flowsBuildCancel covering the scoped/unscoped paths, missing-field default, and RPC rejection propagation. |
| src/openhuman/flows/mod.rs | Single-line addition declaring the new build_registry module as private (pub(crate) visibility handled at the function level). |
Sequence Diagram
sequenceDiagram
participant User
participant ChatComposer
participant useWorkflowBuilderChat
participant flowsApi
participant Tauri/RPC
participant BuildRegistry (Rust)
participant flows_build (Rust)
User->>ChatComposer: Click Send
ChatComposer->>useWorkflowBuilderChat: send(params)
useWorkflowBuilderChat->>useWorkflowBuilderChat: ++sendAttemptRef, setLocalSending(true)
useWorkflowBuilderChat->>flowsApi: buildWorkflow(threadId, ...)
flowsApi->>Tauri/RPC: openhuman.flows_build
Tauri/RPC->>flows_build (Rust): flows_build(req, stream)
flows_build (Rust)->>BuildRegistry (Rust): register_build_turn(thread_id, request_id, token)
flows_build (Rust)->>flows_build (Rust): tokio::select!(run vs token.cancelled())
Note over ChatComposer: isSending=true → Stop button rendered
User->>ChatComposer: Click Stop
ChatComposer->>useWorkflowBuilderChat: stop()
useWorkflowBuilderChat->>flowsApi: flowsBuildCancel(threadId)
flowsApi->>Tauri/RPC: openhuman.flows_build_cancel
Tauri/RPC->>BuildRegistry (Rust): cancel_build_turn_scoped(thread_id, None)
BuildRegistry (Rust)->>flows_build (Rust): token.cancel()
flows_build (Rust)->>flows_build (Rust): select! → Cancelled branch
flows_build (Rust)->>flows_build (Rust): finalize_flow_stream (chat_done event)
flows_build (Rust)->>BuildRegistry (Rust): unregister_build_turn(thread_id, request_id)
flows_build (Rust)->>Tauri/RPC: RpcOutcome (cancelled=true)
Tauri/RPC->>flowsApi: resolves buildWorkflow promise
flowsApi->>useWorkflowBuilderChat: finally block fires
useWorkflowBuilderChat->>useWorkflowBuilderChat: attempt guard passes → setLocalSending(false)
Note over ChatComposer: isSending=false → Send button restored
Reviews (3): Last reviewed commit: "test(flows): cover flowsBuildCancel + st..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/hooks/useWorkflowBuilderChat.ts`:
- Around line 500-516: The stop flow in useWorkflowBuilderChat must not allow a
cancelled buildWorkflow request to update a newer turn. Keep localSending
blocked until the original request settles, or add an attempt/token guard so
only the active send may run endInferenceTurn, update result/proposal/capped
state, and clear localSending; update stop accordingly and add a regression test
covering stop, immediate second send, and late completion of the first request.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fbe70725-d4a2-4faa-a5f2-e1f735a2d594
📒 Files selected for processing (3)
app/src/components/flows/WorkflowCopilotPanel.tsxapp/src/hooks/useWorkflowBuilderChat.test.tsapp/src/hooks/useWorkflowBuilderChat.ts
flows_build runs the workflow_builder agent turn awaited inline (never spawned, to keep with_origin/APPROVAL_CHAT_CONTEXT/thread-id task-local scopes attached), so it can't register in web_chat::IN_FLIGHT or task_dispatcher::ACTIVE_RUNS the way an interactive chat turn or an autonomous run does. Add a dedicated thread_id + request_id scoped registry (mirrors flows::run_registry's register/select shape, and task_dispatcher::cancel_session_scoped's stale-cancel guard) so a real cancel path can reach the running turn.
Race the existing scoped+timeout workflow_builder run future against
the new build_registry token via tokio::select! (both the streaming
and headless branches), instead of only awaiting it inline. A user
Stop now actually interrupts the turn:
- register_build_turn before the run, unregister on every settle path
(ran to completion, errored, timed out, or cancelled) — single call
after the select covers all of them, no early return in between.
- A cancelled turn is treated as a clean stop, not an error: run_error
stays None, the copilot stream gets a chat_done (not chat_error) so
ChatRuntimeProvider ends the inference turn normally, and it returns
before the trail-off backstop (which would otherwise synthesize a
"continue?" question for what is actually a deliberate Stop).
- New ops::flows_build_cancel(thread_id, request_id) delegates to
build_registry::cancel_build_turn_scoped and returns { cancelled }.
The headless (CLI/no-thread) branch keeps its existing behavior
unchanged — there's no thread_id to register a cancellable turn under
in that path.
New flows.build_cancel controller — { thread_id: String, request_id:
Option<String> } -> { cancelled: bool } — delegating to
ops::flows_build_cancel. Registered alongside flows.build in both
all_controller_schemas and all_registered_controllers; updates the two
schema-registry coverage tests to the new 34-controller count.
The Stop button was cosmetic: it called the shared chatCancel (channel_web_cancel), which only ever tears down a spawned interactive chat turn and silently no-ops for flows_build (inline-run, never registered there), while resolving `true` unconditionally. - flowsApi.flowsBuildCancel(threadId, requestId?) calls the new openhuman.flows_build_cancel RPC and returns its real `cancelled` field. - useWorkflowBuilderChat's stop() now calls flowsBuildCancel instead of chatCancel, and no longer eagerly clears `sending` on a confirmed cancel — with real cancellation the in-flight buildWorkflow call now settles promptly on its own, and its own `finally` is the single place `sending` clears. - Attempt-guard (P2/Major re-send race): a useRef counter bumped at the start of every send(); a send's `finally` only clears `sending` when its captured attempt is still the latest one, so an earlier (cancelled/settling) turn can never clear a newer turn's in-flight indicator.
Update the two stop() tests to mock/assert flowsBuildCancel instead of chatCancel, including that stop() no longer eagerly clears `sending` (it now waits for the in-flight buildWorkflow call's own finally). Add a regression test for the attempt-guard: two sends dispatched in the same tick (the stale-closure double-fire the guard exists for) must not let the earlier one's settle clear the newer one's `sending` flag.
|
Want your agent to iterate on Greptile's feedback? Try greploops. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
app/src/hooks/useWorkflowBuilderChat.ts (1)
528-559: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winCancel is unscoped — the backend's
request_idguard is unused here.
flowsBuildCancelsupports an optionalrequestIdto scope the cancel to the exact in-flight turn (matchingcancel_build_turn_scoped's stale-request guard on the server), butstop()never passes one. In practice this means a Stop click always cancels "whatever build turn is currently registered on the thread," not necessarily the specific turn the user is looking at — so the elaborate server-side scoping this PR built is currently moot on the real product path. Threading a client-generated (or server-returned)request_idthroughsend()/buildWorkflow()/stop()would let Stop precisely target the turn it was clicked for.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/hooks/useWorkflowBuilderChat.ts` around lines 528 - 559, Thread the active build turn’s request_id through send() and buildWorkflow() into the hook state, then pass that identifier to flowsBuildCancel from stop(). Ensure the request ID is cleared when the turn settles and that Stop targets only the matching in-flight turn, preserving the existing attempt-guard behavior.src/openhuman/flows/ops.rs (1)
5666-5676: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAdd end-to-end coverage for the
flows_build/flows_build_cancelrace path intests/json_rpc_e2e.rs; the current test only exercises theflows_buildproposal flow.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhuman/flows/ops.rs` around lines 5666 - 5676, Extend the JSON-RPC end-to-end coverage in tests/json_rpc_e2e.rs to exercise the flows_build/flows_build_cancel race, including cancellation before the build completes and validation of the resulting response/outcome. Preserve the existing flows_build proposal-flow assertions and use the BuildRunOutcome::Cancelled behavior as the expected streaming result.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/hooks/useWorkflowBuilderChat.ts`:
- Around line 556-558: Add rejection handling to the fire-and-forget
flowsBuildCancel call in the stop flow, preserving the existing success log
while catching failures and logging the cancellation error with thread context.
Ensure the rejected promise is handled so it does not become an unhandled
rejection.
---
Nitpick comments:
In `@app/src/hooks/useWorkflowBuilderChat.ts`:
- Around line 528-559: Thread the active build turn’s request_id through send()
and buildWorkflow() into the hook state, then pass that identifier to
flowsBuildCancel from stop(). Ensure the request ID is cleared when the turn
settles and that Stop targets only the matching in-flight turn, preserving the
existing attempt-guard behavior.
In `@src/openhuman/flows/ops.rs`:
- Around line 5666-5676: Extend the JSON-RPC end-to-end coverage in
tests/json_rpc_e2e.rs to exercise the flows_build/flows_build_cancel race,
including cancellation before the build completes and validation of the
resulting response/outcome. Preserve the existing flows_build proposal-flow
assertions and use the BuildRunOutcome::Cancelled behavior as the expected
streaming result.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 996584e5-3226-4867-85ed-d0943710bc79
📒 Files selected for processing (7)
app/src/hooks/useWorkflowBuilderChat.test.tsapp/src/hooks/useWorkflowBuilderChat.tsapp/src/services/api/flowsApi.tssrc/openhuman/flows/build_registry.rssrc/openhuman/flows/mod.rssrc/openhuman/flows/ops.rssrc/openhuman/flows/schemas.rs
…ion) flowsBuildCancel is fire-and-forget in stop(); a rejected promise (network/RPC failure) would become an unhandled rejection. Add a .catch that logs the failed cancel attempt -- the in-flight turn's own settle still clears sending.
Restores the diff-coverage gate (was 60%): the copilot Stop FE lines were uncovered because the hook test mocks flowsApi. Adds a flowsApi.test.ts flowsBuildCancel suite (RPC method/params, scoped request_id, cancelled default, rejection) and hook tests for the no-bound-thread guard and the rejected-cancel .catch.
coderabbitai
Bot
added
the
rust-core
1 participant
Summary
stop()fromuseWorkflowBuilderChatand pass it asonStopGenerationto the copilot'sChatComposer(which already had the send↔stop morph logic).Problem
The main chat's
ChatComposershows a Stop button whileisSendingis true and anonStopGenerationhandler is wired (showStopButton), letting the user cancel an in-flight generation. The Workflow Copilot already renders the sameChatComposer, but never passedonStopGeneration— so the Stop button never appeared and there was no way to cancel an in-flight builder turn (which can run for a while, e.g. a 70s+ grounding+build turn).Solution
useWorkflowBuilderChatnow returnsstop(). It cancels the current thread's turn viachatCancel(threadId)— the same primitive the main chat'shandleStopGenerationuses (openhuman.channel_web_cancel). Theflows_buildturn streams over the same web-channel surface (confirmed: it emitsexternal_transfer_pending service=chat-v1on the copilot thread), so the thread-scoped cancel applies.stop()is a no-op when nothing is in flight or no thread is bound, and flipssendingoff on a confirmed cancel for immediate button feedback (the in-flightsendpromise also clears it in itsfinally).WorkflowCopilotPanelpassesonStopGeneration={stop}toChatComposer. No changes toChatComposeritself — it already renders the Stop button whenisSending && onStopGeneration && !hasTypedContent.Submission Checklist
stop cancels the in-flight builder turn via chatCancel and clears sendingandstop is a no-op when nothing is in flightinuseWorkflowBuilderChat.test.ts; existingWorkflowCopilotPanel(36) and hook (32) tests still pass.stoppath is covered by the two new hook tests; the one-line panel prop is exercised by the existing panel render tests. CI coverage gate authoritative.chatCancelservice.Impact
app/srcfiles + a test). No Rust/Tauri change, no new RPC —openhuman.channel_web_cancelalready exists and backs the main chat Stop. No effect on the main chat or any non-copilot surface.Related
AI Authored PR Metadata
Linear Issue
Commit & Branch
feat/copilot-stop-buttondf1b66c253af5aefe497a0a640f4ccf488db1df8Validation Run
pnpm --filter openhuman-app format:check— prettier clean on changed filespnpm typecheck— exit 0, no TS errorsuseWorkflowBuilderChat.test.ts(32 passed) +WorkflowCopilotPanel.test.tsx(36 passed)Validation Blocked
command:N/Aerror:N/Aimpact:N/ABehavior Changes
Parity Contract
ChatComposerunchanged; when noonStopGenerationis passed (every other caller) behavior is identical to before. Main chat Stop untouched.stop()no-ops without a bound thread or in-flight turn.Duplicate / Superseded PR Handling
Summary by CodeRabbit
New Features
Bug Fixes
Tests