fix(web-channel): scope turn cancellation to request_id so a stale cancel cannot kill the next turn (#4760)

[M3gA-Mind]

What

A still-running turn could cause the user's next turn to die at t=0 with a "cancelled" signature — even after the client had moved on. Reported cases: a client that timed out on a long turn and then sent a new message on the same thread saw the new turn resolve as cancelled at t=0; an isolated re-run of the same prompt worked fine (confirming cross-turn interference, not a prompt issue).

Root cause (verified in code)

cancel_chat(client_id, thread_id) / channel_web_cancel cancel whatever turn is currently in IN_FLIGHT[thread], with no request_id scoping — the RPC (channel.web_cancel) and socket (chat:cancel) cancel API carried only client_id + thread_id. So when a client times out on request A and then sends request B (which supersedes A on that thread), A's late, thread-wide cancel tears down B — the newer turn — at t=0. channel_web_cancel compounded it by escalating to the thread-wide task_dispatcher::cancel_session whenever cancel_chat matched nothing, which would also kill a newer turn.

This is exactly the issue's "cancellation isn't scoped to the old request_id." (Not a tinyagents-harness matter: the harness RunContext cancellation token is minted fresh per run — the missing scoping is here in the web-channel routing.)

Fix (minimal)

• cancel_should_target(requested, in_flight) — a small pure predicate: a scoped cancel (Some(request_id)) fires only when it targets the request actually in flight; an unscoped cancel (None) still stops whatever is running.
• cancel_chat_scoped(client_id, thread_id, request_id) built on it — scopes both the primary turn and the request-keyed parallel lane. cancel_chat (2-arg) is kept as the unscoped "stop everything" back-compat entry (Stop button / session teardown), so all existing callers and tests are untouched.
• channel_web_cancel no longer falls through to cancel_session on a scoped no-op — a stale cancel that matched nothing is now a clean no-op instead of a thread-wide teardown.
• Plumbed an optional request_id from the client through the whole path: socket chat:cancel payload → channel.web_cancel RPC schema/params → cancel_chat_scoped. Clients that pass the id of the turn they started now cancel exactly that turn; omitting it preserves today's stop-everything behaviour.

Net effect: a client giving up cleanly cancels its own server turn, and a prior in-flight turn's stale cancel can no longer kill the next one.

Testing

tests/web_cancel_request_scoping.rs pins the scoping decision: unscoped cancels always target the in-flight turn; a scoped cancel fires only for its own request; a stale scoped cancel for a superseded request is a no-op (the newer turn survives). It's an integration test of the pure predicate, so it's fast, deterministic, and independent of any turn/config setup. No local build (disk constrained on this runner); CI compiles/runs it.

Closes #4760

Summary by CodeRabbit

• New Features

  • Cancellation requests can now optionally include a request ID to target a specific chat turn (instead of canceling the whole in-flight activity on the thread).
  • This scoped behavior also applies to parallel turns when applicable.

• Bug Fixes

  • Late/stale scoped cancels no longer interrupt newer in-flight turns.
  • Scoped cancels that don’t match the active request are now safely ignored, avoiding unintended broader teardown.

• Tests

  • Added coverage to verify request-scoped cancellation targeting and non-interference from stale cancels.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5272c798d1

ℹ️ 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".

[coderabbitai]

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 48 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ec78804e-8870-4a24-b2d8-65b3aee64629

📥 Commits

Reviewing files that changed from the base of the PR and between fcecc0c and c40dced.

📒 Files selected for processing (4)

• src/openhuman/agent/task_dispatcher/mod.rs
• src/openhuman/agent/task_dispatcher/registry.rs
• src/openhuman/agent/task_dispatcher/tests.rs
• src/openhuman/channels/providers/web/ops.rs

📝 Walkthrough

Walkthrough

This PR adds request-scoped cancellation for web-channel chat turns. ChatCancelPayload, WebCancelParams, and the web cancel schema gain an optional request_id. channel_web_cancel and a new cancel_chat_scoped function use this to target only the matching in-flight primary or parallel turn, ignoring stale cancels, while cancel_chat retains prior unscoped behavior. Tests and call sites are updated accordingly.

Changes

Request-scoped cancellation

Layer / File(s)	Summary
Payload and socket handler src/core/socketio.rs	ChatCancelPayload gains optional request_id; the chat:cancel handler forwards it to cancel_chat_scoped instead of cancel_chat.
Web cancel types and schema src/openhuman/channels/providers/web/types.rs, src/openhuman/channels/providers/web/schemas.rs	WebCancelParams and the "cancel" controller schema gain optional request_id; handle_cancel forwards it to channel_web_cancel.
Scoped cancellation logic src/openhuman/channels/providers/web/ops.rs, src/openhuman/channels/providers/web/mod.rs	Adds cancel_should_target and cancel_parallel_turn_by_request_id; reworks cancel_chat_scoped to target only matching primary/parallel turns and ignore stale scoped cancels; cancel_chat delegates with None; channel_web_cancel accepts request_id and skips session teardown on unmatched scoped cancels; new functions re-exported.
Tests tests/channels_web_startup_raw_coverage_e2e.rs, tests/tools_network_channels_raw_coverage_e2e.rs, tests/web_cancel_request_scoping.rs	Existing test call sites updated for the new request_id argument; new test file validates cancel_should_target matching for scoped/unscoped/stale cancels.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant SocketHandler as chat:cancel handler
  participant ChannelWebCancel as channel_web_cancel
  participant CancelChatScoped as cancel_chat_scoped
  participant TurnRegistry as In-flight turn registry

  Client->>SocketHandler: chat:cancel(request_id?)
  SocketHandler->>CancelChatScoped: cancel_chat_scoped(client_id, thread_id, request_id)
  CancelChatScoped->>TurnRegistry: lookup in-flight primary/parallel turns
  TurnRegistry-->>CancelChatScoped: in_flight request_id(s)
  CancelChatScoped->>CancelChatScoped: cancel_should_target(requested, in_flight)
  alt matches or unscoped
    CancelChatScoped->>TurnRegistry: cancel matching turn(s)
    CancelChatScoped-->>ChannelWebCancel: Some(cancelled_id)
  else stale scoped cancel
    CancelChatScoped-->>ChannelWebCancel: None (ignored, logged)
  end
  ChannelWebCancel-->>SocketHandler: RpcOutcome(cancelled)

Loading

Suggested labels: rust-core, bug

Suggested reviewers: sanil-23

Poem

A rabbit hops with cancel in paw,
No more stale cancels breaking the law! 🐇
Each turn now guarded by its own ID,
Stale ones ignored, newer ones freed.
Threads run true, no cross-thread strife—
Hop hop hooray for a scoped little life! 🥕

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: scoping web-channel turn cancellation by request_id to prevent stale cancels from killing newer turns.
Linked Issues check	✅ Passed	The PR implements request-scoped web cancellation, preserves unscoped cancel behavior, avoids session teardown on stale scoped cancels, and adds regression coverage for the reported bug.
Out of Scope Changes check	✅ Passed	The changes stay focused on web-channel cancellation scoping, API plumbing, exports, and tests, with no clear unrelated additions.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/openhuman/channels/providers/web/ops.rs (1)

1002-1066: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

cancel_chat_scoped return value ignores parallel-turn cancellations, causing channel_web_cancel to misreport cancelled: false.

removed_request_id only reflects the primary in-flight turn. When a scoped cancel matches only a parallel (forked) turn — cancel_parallel_turn_by_request_id succeeds and a "chat_error"/cancelled event is actually published for it — the function still returns Ok(None). Downstream, channel_web_cancel (Line 1180-1186) treats cancelled_request_id.is_none() && request_id.is_some() as "matched nothing" and reports cancelled: false, request_id: null, even though the parallel turn was genuinely cancelled. The RPC ack becomes inconsistent with the emitted event.

🐛 Proposed fix to fold parallel cancellations into the returned id

     let cancelled_parallel = match request_id {
         Some(rid) => cancel_parallel_turn_by_request_id(thread_id, rid).await,
         None => cancel_parallel_turns_for_thread(thread_id).await,
     };

+    let cancelled_any = removed_request_id
+        .clone()
+        .or_else(|| cancelled_parallel.first().cloned());
+
     // Emit a cancelled chat_error for each cancelled turn (primary + parallels)
     // so every interleaved branch's UI is resolved.
     for request_id in removed_request_id.iter().cloned().chain(cancelled_parallel) {
         publish_web_channel_event(WebChannelEvent {
             event: "chat_error".to_string(),
             client_id: client_id.to_string(),
             thread_id: thread_id.to_string(),
             request_id,
             message: Some("Cancelled".to_string()),
             error_type: Some("cancelled".to_string()),
             ..Default::default()
         });
     }

-    Ok(removed_request_id)
+    Ok(cancelled_any)
 }

This also fixes the "request_id": cancelled_request_id field in channel_web_cancel's JSON response (Line 1188-1196), which currently omits a cancelled parallel turn's id.

🤖 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/channels/providers/web/ops.rs` around lines 1002 - 1066,
`cancel_chat_scoped` only returns the primary in-flight request id, so scoped
cancels that hit a parallel turn are reported as if nothing matched. Update
`cancel_chat_scoped` to capture the id(s) returned by
`cancel_parallel_turn_by_request_id` and include them in the function’s
`Ok(...)` result when no primary turn was removed. Keep the existing
`publish_web_channel_event` loop intact, and make sure `channel_web_cancel`’s
`cancelled_request_id`/`cancelled` response reflects parallel-turn cancellations
as well.

🤖 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.

Outside diff comments:
In `@src/openhuman/channels/providers/web/ops.rs`:
- Around line 1002-1066: `cancel_chat_scoped` only returns the primary in-flight
request id, so scoped cancels that hit a parallel turn are reported as if
nothing matched. Update `cancel_chat_scoped` to capture the id(s) returned by
`cancel_parallel_turn_by_request_id` and include them in the function’s
`Ok(...)` result when no primary turn was removed. Keep the existing
`publish_web_channel_event` loop intact, and make sure `channel_web_cancel`’s
`cancelled_request_id`/`cancelled` response reflects parallel-turn cancellations
as well.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f9d90808-563d-4889-a269-03206fe482b5

📥 Commits

Reviewing files that changed from the base of the PR and between 54511e6 and 57e7c6f.

📒 Files selected for processing (8)

• src/core/socketio.rs
• src/openhuman/channels/providers/web/mod.rs
• src/openhuman/channels/providers/web/ops.rs
• src/openhuman/channels/providers/web/schemas.rs
• src/openhuman/channels/providers/web/types.rs
• tests/channels_web_startup_raw_coverage_e2e.rs
• tests/tools_network_channels_raw_coverage_e2e.rs
• tests/web_cancel_request_scoping.rs

[M3gA-Mind]

CI guardian: ran cargo fmt on tests/web_cancel_request_scoping.rs (the only cargo fmt --check diff) — fixes the Rust Quality fmt failure.

[M3gA-Mind]

Addressed the outstanding bot review comments in f21f498 (pushed):

• @coderabbitai (Major, ops.rs cancel_chat_scoped) — parallel-turn cancellations were dropped from the return value, so a scoped cancel that matched only a forked turn made channel_web_cancel misreport cancelled: false despite emitting a cancelled event. Now folds the cancelled parallel id into the result so the RPC ack matches the emitted event.
• @chatgpt-codex-connector (P2, task-dispatcher) — a scoped cancel targeting an autonomous run was suppressed by the request_id.is_some() => false branch, leaving the run alive. New request-aware cancel_session_scoped aborts the run only on a matching run_id; channel_web_cancel routes its fallback through it. Unit tests added for match / mismatch / no-run.
• @chatgpt-codex-connector (P2, desktop cancel path) — deferred to a follow-up with rationale in the inline thread: the backend scoping is already exercised by the socket.io chat:cancel path, and threading request_id through the desktop Stop button would turn it into a no-op once a newer turn starts (an unscoped-vs-scoped UX decision out of scope here).

cargo fmt clean; leaving compile/test verification to CI.

[chatgpt-codex-connector]

To use Codex here, create a Codex account and connect to github.

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail
cd /tmp
rm -rf repo
git clone --depth 50 https://github.com/tinyhumansai/openhuman repo 2>&1 | tail -5
cd repo
git fetch origin pull/4765/head:pr4765 2>&1 | tail -5
git checkout pr4765 2>&1 | tail -5
echo "--- log ---"
git log --oneline -10
echo "--- ops.rs relevant section ---"
rg -n "cancel_chat_scoped|cancel_should_target|cancel_parallel_turn_by_request_id|channel_web_cancel" src/openhuman/channels/providers/web/ops.rs

Length of output: 2465

---

🏁 Script executed:

#!/bin/bash
cd /tmp/repo
echo "=== commit 499f4db ==="
git show 499f4db -- src/openhuman/channels/providers/web/ops.rs
echo "=== commit f21f498 ==="
git show f21f498

Length of output: 11199

---

Both fixes look correct and well-targeted.

• Parallel-turn fix (499f4db): cancelled_any now falls back to cancelled_parallel.first().cloned() when the primary removed_request_id is None, so a scoped cancel that only tore down a forked turn correctly returns Some(id) instead of None. This lines up with the emitted chat_error events (still iterating removed_request_id.into_iter().chain(cancelled_parallel)), so the RPC ack (cancelled: true) now matches what was actually published.
• Task-dispatcher fix (f21f498): cancel_session_scoped peeks the active run (via lock, without removing it) and only proceeds to cancel_session when run_id matches the requested request_id, otherwise no-ops and leaves the run intact — closing the gap where channel_web_cancel's old else if request_id.is_some() => false branch silently dropped scoped cancels aimed at autonomous runs. The three added unit tests (no run, mismatched request, matching request) cover the relevant branches, including verifying the card lands in Blocked on a successful scoped cancel.

Both changes preserve the unscoped "stop everything" semantics (request_id: None still behaves exactly as before) while closing the two gaps flagged. Nothing further from me on these two points — nice fixes.

[resolve]

[YellowSnnowmann]

Review: approve

Feature satisfied. The PR scopes web-channel turn cancellation to request_id end-to-end and closes the real #4760 mechanism (a same-thread late cancel for request A tearing down the newer request B). Verified against the PR head, not just the diff.

What holds up:

• cancel_should_target (ops.rs:948) is a clean pure predicate: scoped cancel fires only on rid == in_flight.request_id; None keeps the unscoped "stop everything" path. #[serde(default)] on the optional request_id in ChatCancelPayload/WebCancelParams preserves back-compat (absent = None = today's behaviour).
• Coverage is complete across all three lanes: primary IN_FLIGHT turn, matching parallel/forked turn (cancel_parallel_turn_by_request_id), and detached autonomous runs (cancel_session_scoped, registry.rs:83, aborts only on run_id match).
• The socket consumer is fully wired: chat:start emits chat_accepted { request_id } (socketio.rs:526), which the client passes back on chat:cancel.
• Error handling is correct: a stale scoped cancel logs at info and no-ops; cancellation surfaces a structured chat_error {error_type: "cancelled"} event, not a Sentry capture — right per the repo's error-taxonomy rules.

Note on the issue's "cross-thread" symptom: IN_FLIGHT is keyed purely by thread_id (key_for returns it verbatim), so there is no global cancellation state that could kill a turn on a different thread. The evidence in #4760 describing cross-thread death looks like a misdiagnosis; the actually-fixable mechanism is the same-thread A-supersedes-B race, which the PR body correctly identifies and this diff fixes.

Minor / non-blocking:

• The desktop Stop button (chatService.ts, unchanged here) still sends request_id=None, so scoping is currently exercised only via the socket path. Deferring this is reasonable — threading the id into an unscoped Stop button would make it a silent no-op once a newer turn starts. Fine as a tracked follow-up.
• cancel_session_scoped peeks the run under the std mutex, drops it, then cancel_session re-takes it. The window is benign: cancel_session returns false if the run already completed, and a same-thread run swap in that microsecond window is not realistic. No change needed.

Review threads: all three substantive threads are genuinely addressed — the parallel-turn return-value major (folded into cancelled_any, confirmed by CodeRabbit), the task-dispatcher scoped fallback (landed in f21f498 with unit tests), and the desktop-wiring deferral (rationale sound). CI is green (the earlier fmt failure is fixed).

[YellowSnnowmann]

Approving — feature satisfied, no blocker/major issues, review comments addressed in code, CI green. Detailed review in the PR comment above.