openjd-sessions: cross-user session silently drops external SessionConfig.cancel_token cancel

[crowecawcaw]

Summary

On a cross-user session, cancelling via the external SessionConfig.cancel_token (a bare token.cancel(), not Session::cancel_action) has no effect: the running action's child process is never signalled and the action is reported Success/Failed as if no cancel happened. The same operation works on same-user sessions.

Confirmed against main @ 99871a0293ef0ebfb791cdef6d38c639914463db. Crate: openjd-sessions (src/cross_user_helper.rs, run_via_helper).

Root cause

Cancellation reaches a running action over two channels:

1. the tokio::sync::watch channel (cancel_request_rx), which Session::cancel_action fires and mirrors onto the cross-user helper's stdin via cancel_writer; and
2. the per-action CancellationToken — a child of SessionConfig.cancel_token (new_action_cancel_token() = parent.child_token()).

Session::cancel_action (and SessionCancelHandle::cancel) fire both. A bare external SessionConfig.cancel_token.cancel() fires only the token — it sends nothing on the watch channel and writes nothing to cancel_writer.

• The same-user loop (src/subprocess.rs) awaits cancel_token.cancelled() directly and re-checks the sticky cancel_token.is_cancelled() in the final-state computation, so it observes a cancel from either channel.
• The cross-user run_via_helper (src/cross_user_helper.rs) select loop has only two branches — the helper stdout response and the timeout — and never awaits the token. Its cancel classification is watch-channel-only:

  let canceled = config.cancel_request_rx.as_ref()
      .is_some_and(|rx| rx.has_changed().unwrap_or(false));

  run_subprocess_via_helper registers the token via set_action but passes it only there — run_via_helper receives no token and cannot act on it.

So a bare parent-token cancel of a cross-user session: marks the child token cancelled, tells the helper nothing, and at exit has_changed() is false → the action resolves to Success/Failed, never Canceled.

Note on the recent partial fix

A recent change made the cross-user path honor the watch-channel cancel, which closes the cancel_action / SessionCancelHandle::cancel cross-user path. This issue is the remaining gap: the token-only external cancel is still dropped.

Suggested fix

Make run_via_helper observe the token too, mirroring the same-user loop:

• thread the per-action CancellationToken into run_via_helper and add a select! branch on cancel_token.cancelled() that writes the tokenized cancel command to the helper over cancel_writer; and
• include cancel_token.is_cancelled() in the final-state classification (i.e. is_cancelled() || cancel_request_rx.has_changed()).

Reproduction (conceptual)

1. Create a cross-user Session (user: Some(...)) with SessionConfig.cancel_token = Some(tok).
2. Start a long-running action.
3. From another task, call tok.cancel() directly (not session.cancel_action(...)).
4. Expected: the action terminates and is reported Canceled. Actual: the child runs to completion; the action reports Success/Failed.

Reachability

specs/sessions/session.md documents SessionConfig.cancel_token as the mechanism to "cancel an entire session from outside the session's async context (e.g., when the Deadline service sends a cancel request)." The bundled openjd CLI is not affected: its SIGINT/SIGTERM handler does call a bare cancel_token.cancel(), but it runs same-user (user: None), so the same-user loop observes the token. The exposed consumer is an external cross-user host (worker agent) that constructs a session with both user: Some(...) and cancel_token: Some(...) and relies on the bare parent token for session cancel. A cross-user consumer that cancels via cancel_action / SessionCancelHandle instead is unaffected.

How it was found

Surfaced by a formal (P) model of the sessions runtime that models cancellation as two-channel delivery and asserts an issued cancel is never dropped; the model checker produces a counterexample on the cross-user token-only path. Verified by two independent adversarial code reviews against the sha above. A candidate fix with #[cfg(unix)] mocked-helper unit tests exists but has not yet been exercised against a real cross-user (Docker/root) helper.