fix(python): avoid closed loop completion callbacks

[willkill07]

Overview

Prevents Python tool execution from queuing a completion callback after its owning asyncio event loop has closed during cancellation.

• I confirm this contribution is my own work, or I have the right to submit it under this project's license.
• I searched existing issues and open pull requests, and this does not duplicate existing work.

Details

• Replace the generic PyO3 completion bridge for managed tool execution with a loop-aware bridge.
• Cancel the Rust task when its Python future is cancelled.
• Skip completion scheduling when the owning event loop is closed.

Where should the reviewer start?

Start with crates/python/src/py_api/mod.rs, especially safe_future_into_py and its cancellation/completion callbacks.

Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)

• Relates to fix: add offline mode to doctor probes #670

Summary by CodeRabbit

• Bug Fixes
  • Improved cancellation handling for Python asynchronous operations.
  • Prevented callbacks from running against cancelled or closed event loops.
  • Ensured operation results and errors are delivered safely through the Python event loop.
  • Tool execution now responds more reliably when interrupted or cancelled.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: a433e721-bfda-42e2-b868-4ec7ac23260f

📥 Commits

Reviewing files that changed from the base of the PR and between e52d016 and 3281233.

📒 Files selected for processing (1)

• crates/python/src/py_api/mod.rs

📜 Recent review details

🧰 Additional context used

📓 Path-based instructions (11)

**/*.rs

📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)

**/*.rs: Any Rust change must run just test-rust
Any Rust change must run cargo fmt --all
Any Rust change must run cargo clippy --workspace --all-targets -- -D warnings

**/*.rs: Run cargo fmt --all for all FFI work since it is Rust work
Run just test-rust to validate FFI changes
Run cargo clippy --workspace --all-targets -- -D warnings to enforce strict linting on FFI work

When Rust files changed as part of Go work, also run cargo fmt --all, just test-rust, and cargo clippy --workspace --all-targets -- -D warnings

**/*.rs: Run cargo fmt --all when Rust files are changed as part of Node work
Run cargo clippy --workspace --all-targets -- -D warnings when Rust files are changed as part of Node work
Run just test-rust when Rust files are changed as part of Node work

When changing the core Rust runtime or Rust-facing API surface, format Rust code with cargo fmt (rustfmt defaults), keep cargo clippy -- -D warnings clean, and satisfy cargo deny check per deny.toml.

**/*.rs: If any Rust code changed, always run just test-rust.
If any Rust code changed, also run cargo fmt --all.
If any Rust code changed, also run cargo clippy --workspace --all-targets -- -D warnings.
For Rust changes headed for review, run cargo fmt --all and cargo clippy --workspace --all-targets -- -D warnings even if relying on pre-commit.

Files:

• crates/python/src/py_api/mod.rs

**/*.{rs,py}

📄 CodeRabbit inference engine (AGENTS.md)

Follow binding naming conventions in Rust and Python: use snake_case.

Files:

• crates/python/src/py_api/mod.rs

**/*.{rs,py,js,mjs,cjs,ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{rs,py,js,mjs,cjs,ts,tsx}: Use Json = serde_json::Value in Rust-facing runtime APIs where the existing code expects JSON payloads.
Use Result<T> with FlowError in core runtime paths, and keep errors explicit and binding-appropriate at the wrapper layer.
Keep async behavior on the existing tokio-based model; bindings should preserve callback and future lifetimes rather than blocking or hiding async work unexpectedly.

Files:

• crates/python/src/py_api/mod.rs

**/*.{rs,py,go,js,ts,c,h}

📄 CodeRabbit inference engine (CONTRIBUTING.md)

Use language-appropriate naming conventions: Rust snake_case, C FFI exports prefixed nemo_relay_, Go PascalCase, Node.js camelCase, and Python snake_case.

Files:

• crates/python/src/py_api/mod.rs

**/*.{rs,go,js,ts}

📄 CodeRabbit inference engine (CONTRIBUTING.md)

Add the SPDX license header to all Rust, Go, JavaScript, and TypeScript source files using the corresponding // comment form.

Files:

• crates/python/src/py_api/mod.rs

{crates/python/src/py_api/mod.rs,python/nemo_relay/**/*.py,python/nemo_relay/**/*.pyi,go/nemo_relay/**/*.go,crates/node/src/api/**/*.rs}

📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)

Update the language-native bindings for every exposed surface in Python, Go, and Node.js.

Files:

• crates/python/src/py_api/mod.rs

{crates/**/src/**/*.rs,python/**/*.py}

📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)

Do not add tests under src; Rust tests belong in crate tests/ trees, and Python SDK tests belong under python/tests.

Files:

• crates/python/src/py_api/mod.rs

**/*

📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)

**/*: Format changed files with the language-native formatter before the final lint/test pass.
If dynamic plugin behavior changed, use maintain-dynamic-plugins and include the native SDK, worker protocol, Python SDK, docs, packaging, and Codecov surfaces in the validation plan.
If code changes alter APIs, bindings, commands, paths, packaging behavior, observability/adaptive semantics, or documented best practices, update any dependent maintainer or consumer skills in the same branch.
During iteration, prefer uv run pre-commit run --files <changed files...>.
Before review or handoff, run uv run pre-commit run --all-files.

Files:

• crates/python/src/py_api/mod.rs

**/*.{rs,py,go,js,ts}

📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)

If a language surface changed, always run that language's test target even when Rust core did not change.

**/*.{rs,py,go,js,ts}: When observability configuration or lifecycle is exposed, keep FFI and Python, Go, and Node.js binding-native config objects and subscriber/exporter methods aligned in logical knobs and semantics.
Require every OpenTelemetry endpoint to have a type and nonblank destination; resolve header_env values at activation and reject missing, blank, or duplicate headers.
Concatenate layered ATOF sink, ATIF storage, and OpenTelemetry endpoint lists with higher-precedence entries first.
Preserve correct handling of mark events, start/end events, orphan cases, and span or trajectory fields derived from intended event data.
Run affected Rust tests and just test-rust when event fields change; run just test-python, just test-go, and just test-node when binding-native configuration or lifecycle changes.

Files:

• crates/python/src/py_api/mod.rs

**/*.{rs,py,js,ts,tsx,go,java,kt,swift}

📄 CodeRabbit inference engine (.agents/skills/add-middleware/SKILL.md)

Add tests covering registration and duplicate names, deregistration and missing names, priority ordering, callback failure policy, scope-local inheritance and cleanup, event payload semantics, immutable mark and scope fields, and parity across affected bindings.

Files:

• crates/python/src/py_api/mod.rs

crates/{python,ffi,node}/**/*

⚙️ CodeRabbit configuration file

crates/{python,ffi,node}/**/*: Treat binding changes as public API changes. Check for parity with the other language bindings, FFI ownership/lifetime safety,
callback error propagation, stable type conversion, and consistent async/stream semantics.
Flag changes that update one binding without corresponding tests or documentation for the same surface elsewhere.

Files:

• crates/python/src/py_api/mod.rs

🧠 Learnings (1)

📚 Learning: 2026-08-03T19:55:03.931Z

Learnt from: afourniernv
Repo: NVIDIA/NeMo-Relay PR: 558
File: crates/pii-redaction/src/rampart/mod.rs:265-274
Timestamp: 2026-08-03T19:55:03.931Z
Learning: In NeMo Relay first-party plugin registration helpers, treat the documented duplicate-registration `PluginError::RegistrationFailed` result from `register_plugin` as success when registration is intended to be idempotent. Do not locally reclassify this as `PluginError::Conflict`; changing the classification requires a core-wide review of the public API and FFI behavior.

Applied to files:

• crates/python/src/py_api/mod.rs

🔇 Additional comments (3)

crates/python/src/py_api/mod.rs (3)

44-44: LGTM!

---

801-801: LGTM!

---

73-153: 📐 Maintainability & Code Quality

Run the required validation commands separately and attach their results before merge. The combined validation command timed out, so no validation status is available.

---

Walkthrough

The Python API adds a cancellation-aware bridge for Rust futures. It creates native asyncio futures, cancels Rust tasks when Python futures are cancelled, and safely propagates results or exceptions. Tool execution uses the new bridge.

Changes

Python future bridge

Layer / File(s)	Summary
Future cancellation and completion bridge crates/python/src/py_api/mod.rs	Adds cancellation callbacks, cancellation-aware Rust future handling, event-loop closure checks, and thread-safe result or exception completion.
Tool execution integration crates/python/src/py_api/mod.rs	Routes tool execution through safe_future_into_py and adds the required PyO3 conversion import.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant PythonFuture
  participant SafeFutureCanceller
  participant RustTask
  participant EventLoop
  PythonFuture->>SafeFutureCanceller: cancellation callback
  SafeFutureCanceller->>RustTask: cancellation signal
  RustTask->>EventLoop: result or exception
  EventLoop->>PythonFuture: complete future

Loading

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title follows Conventional Commits format, uses an allowed type and scope, stays under 72 characters, and summarizes the change.
Description check	✅ Passed	The description includes all required sections, completed contribution checks, implementation details, reviewer guidance, and a valid related issue reference.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[mnajafian-nv]

LGTM

[willkill07]

/merge