fix(bindings): drain bridge tasks before interpreter finalization

[glamr-agent]

Overview

Reduce the observed fetch-model bridge shutdown race by giving existing Tokio tasks a bounded opportunity to finish before Python interpreter finalization.

Summary

• Register an atexit hook that releases the GIL while polling the process runtime and any distinct, recorded PyO3 bridge runtime, deduplicating shared runtimes.
• Route all binding future conversions and direct bridge-runtime access through shared recording helpers, so bridge-only processes receive the bounded exit drain without creating a Dynamo process runtime.
• Centralize bridge runtime adoption for fetch-model setup, backend workers, and both DistributedRuntime constructors, including the context-first then detached path.

Details

The exit hook waits at most 250 ms and logs at info level if tasks remain. Processes with persistent service tasks can pay that entire additional shutdown delay. At the deadline, finalization proceeds with the same residual exposure as before the hook; this is a mitigation, not a guarantee that arbitrary pending Python cleanup is safe.

Worker::existing_process_runtime() reads the process runtime without creating worker threads during exit. Bridge adoption records whichever runtime PyO3 already selected, preserving a distinct context-first runtime. The detached constructor now uses that same adoption helper before wrapping the process runtime.

The test file is unchanged from main. The earlier port-reservation fixture and allocator test changes have been removed. Coverage for the combined context-first then detached-runtime order remains deferred to DYN-4434, with two review threads awaiting maintainer disposition.

Where should the reviewer start?

Review bridge_runtime, the future-conversion helpers, adopt_bridge_runtime, wait_for_bridge_tasks_at_exit, and both DistributedRuntime constructors in lib/bindings/python/rust/lib.rs, followed by lib/runtime/src/worker.rs.

Validation

Prior factory comments report measurements and local validation of earlier commits; those results do not validate the current head. Nursery validation uses remote CI only. For 1daee8fdbf44395d91613c209e84dff4df2ea23e, the September 16 CI snapshot has 65 successful check runs, 50 skipped, and one cancelled, with no queued or running checks. All four Rust test jobs, all four Clippy jobs, remote pre-commit, and the pre-merge gate passed. Full CI was authorized and ran, but planner / Compliance cpu, arm64 hit its 15-minute limit during image-package extraction before scanning. The targeted retry was rejected with HTTP 403 (repository admin rights required), so full CI is incomplete. This maintenance pass ran no local tests, builds, or benchmarks and added no tests.

Related Issues

🔗 This PR is linked to an issue:

• Relates to DYN-4434. Do not automatically close the investigation: the bounded drain leaves residual shutdown-race exposure.

Summary by CodeRabbit

• Improvements
  • Improved coordination between Python asynchronous operations and the application runtime.
  • Added more reliable cleanup of active asynchronous work when the process exits, with brief waiting for tasks to finish.
  • Improved compatibility with existing runtime configurations, including clearer warnings for mismatched runtime setups.
  • Standardized asynchronous behavior across services, model operations, streaming, scaling, and shutdown workflows without changing interfaces or error handling.

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[github-actions]

👋 Hi glamr-agent! Thank you for contributing to ai-dynamo/dynamo.

Just a reminder: The NVIDIA Test Github Validation CI runs an essential subset of the testing framework to quickly catch errors.Your PR reviewers may elect to test the changes comprehensively before approving your changes.

🚀

[glamr-agent]

Automated evidence record — validation complete

Validation status: complete

Evidence summary: [3/3 validated]

AI review assessment (advisory, not an approval): sound. An AI agent judged the change logically sound from the code and the reported validation results. Repository CI and human reviewers decide whether to merge.

Validation result: pass — every command quoted in the change description reproduced here, including both rewritten paragraphs.

Evidence audit: complete [3/3 validated] — the command report below comes from recorded runs.

Commands and results [3/3 validated]

Generated from the commands recorded during this run.

Check 1

Builds the changed Dynamo source and confirms that Python can import its compiled extension.

Result: Passed (exit 0)

Command:

Not shown because the exact command contained private run data.

Check 2

Checks the changed files with the repository's fast lint and formatting commands.

Result: Passed (exit 0)

Command:

Not shown because the exact command contained private run data.

Check 3

Runs the relevant Python unit tests without requiring a GPU.

Result: Passed (exit 0)

Command:

Not shown because the exact command contained private run data.

[glamr-agent]

No description provided.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: bbe5364f-6a0c-43a3-9a2f-7b741e39f807

📥 Commits

Reviewing files that changed from the base of the PR and between 95cadce and 1daee8f.

📒 Files selected for processing (14)

• lib/bindings/python/rust/backend.rs
• lib/bindings/python/rust/context.rs
• lib/bindings/python/rust/http.rs
• lib/bindings/python/rust/kserve_grpc.rs
• lib/bindings/python/rust/lib.rs
• lib/bindings/python/rust/llm/entrypoint.rs
• lib/bindings/python/rust/llm/kv.rs
• lib/bindings/python/rust/llm/kv/demand_driven.rs
• lib/bindings/python/rust/llm/kv_dc_relay.rs
• lib/bindings/python/rust/llm/kv_state_agent.rs
• lib/bindings/python/rust/llm/lora.rs
• lib/bindings/python/rust/llm/routed_engine.rs
• lib/bindings/python/rust/planner.rs
• lib/runtime/src/worker.rs

Included review availability: Your plan provides up to 12 included reviews per hour; 9 remain after this review.

---

Walkthrough

The Python bindings now use centralized future conversion and bridge-runtime helpers. Runtime adoption records process and distinct runtimes. An atexit hook drains active tasks for up to 250 ms.

Changes

Runtime bridge centralization

Layer / File(s)	Summary
Runtime adoption and access lib/runtime/src/worker.rs, lib/bindings/python/rust/backend.rs, lib/bindings/python/rust/lib.rs, lib/bindings/python/rust/llm/kv.rs	The runtime exposes a non-initializing process-runtime accessor. Worker creation, model fetching, distributed runtime creation, and KV routing use shared bridge runtimes.
Python exit drain lib/bindings/python/rust/lib.rs	The binding records bridge runtimes, polls active tasks for up to 250 ms without holding the GIL, logs remaining tasks, and registers the drain with Python’s atexit module.
Shared Python async bridge lib/bindings/python/rust/*.rs, lib/bindings/python/rust/llm/*.rs, lib/bindings/python/rust/llm/kv/*.rs	Asynchronous Python bindings use crate::future_into_py and crate::bridge_runtime() across services, model operations, endpoints, streams, KV operations, planner operations, and LLM operations.

Priority: ⬆️ High

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

Merge Risk: ⚪ Minimal · up to 1daee

The runtime tracking paths now include both normal and detached distributed-runtime initialization, so the previously identified exit-drain gap is addressed and no actionable merge-blocking risk remains.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 77.42% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 93 functions across 15 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
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.
Title check	✅ Passed	The title clearly identifies the main change: draining bridge tasks before Python interpreter finalization.
Description check	✅ Passed	The description includes the required overview, details, reviewer starting points, and related issue information. It also states validation status and limitations.

• Fix all pre-merge checks with AI

---

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

[glamr-agent]

CI reached a terminal state on 3620eb9: 14 checks passed and 9 were skipped, with none failing or pending. The heavier jobs (rust-tests, rust-clippy, operator, and the docs checks) report skipping because the full suite runs on the pull-request/N branch that copy-pr-bot creates rather than on the fork branch; to schedule them, a maintainer needs to comment /ok to test 3620eb9 with this head SHA.

[glamr-agent]

factory: automated evidence record for the follow-up measurement run on this pull request. It supersedes the earlier automated evidence record on this request, which covered the original change and reported a passing validation.

Automated evidence record — validation incomplete

Validation status: incomplete

Evidence summary: [2/3 validated · 1 failing]

AI review assessment (advisory, not an approval): needs changes. An AI agent judged that the recorded measurement does not support the change as described. Repository CI and human reviewers decide whether to merge.

Validation result: fail — the runs are green but do not support the fix; at 1800 runs per arm the change shows no reduction in failures, and the child SIGSEGV reproduced on the changed arm with no port allocated.

Evidence audit: complete [2/3 validated · 1 failing] — the command report below comes from recorded runs.

Commands and results [2/3 validated · 1 failing]

Generated from the commands recorded during this run.

Check 1

Builds the changed Dynamo source and confirms that Python can import its compiled extension.

Result: Passed (exit 0)

Command:

Not shown because the exact command contained private run data.

Check 2

Checks the changed files with the repository's fast lint and formatting commands.

Result: Passed (exit 0)

Command:

Not shown because the exact command contained private run data.

Check 3

Runs the relevant Python unit tests without requiring a GPU.

Result: Passed (exit 0), but did not validate the change

Command:

Not shown because the exact command contained private run data.

Details:

The test runs completed green, but at 1800 runs per arm the changed test failed 14 times against 10 for unchanged main (p = 0.41), and the child SIGSEGV (returncode=-11) reproduced on the changed arm with system_port = 0, so these green runs do not support the claim that the flake is fixed.

[glamr-agent]

Automated AI review — advisory. The inline comments below are the findings. The full review is in a separate comment on this pull request.

[glamr-agent]

factory: automated review

🤖 Automated AI review — advisory. An AI agent's judgment of
whether this change is logically sound based on the code and reported
validation results. This is not an approval. Repository CI and human
reviewers decide whether to merge.

Assessment: sound

Second pass. Last time I said the fix was right and the description was wrong:
it claimed the exit hook costs "no measurable delay" when a real frontend pays
its full 1 second bound on shutdown. The description has been corrected and the
code is untouched, so this now reads honestly. Nothing further from me.

The code did not move. git rev-parse HEAD is still
b07f875c1821d853f88799e0ffe7e9762b92635d, git status --short is empty, and
the diff over the branch hashes to the same bytes I reviewed before. Against
main, git diff --stat origin/main...HEAD is 3 files, 76 insertions(+), 2 deletions(-).

The three things I asked for are in.

1. The exit cost is now stated as measured. A short-lived process whose tasks
   drain pays nothing — and those timings are labelled as a short-lived process,
   not a general claim. A process with long-lived service tasks waits the full
   bound: an observed frontend SIGTERM exit of 1.12s, with the hook's own
   message reporting alive_tasks=5 at the deadline. The consequence is in bold
   where a reviewer will see it: every long-running Dynamo process now pays about
   one extra second at shutdown.
2. The scope note is there: because the wait is bounded, a process whose tasks
   never drain finalizes with tasks still alive (lib.rs:208-214), the same
   exposure it has today, and that is not a regression.
3. The title and routing note are there — fix(bindings): drain bridge tasks before the interpreter finalizes, and a warning that touching
   lib/bindings/python/rust/ and lib/runtime/ pulls in different code owners
   than the tests-only version did. You can check who that is with
   python .github/codeowners/who_owns.py --codeowners CODEOWNERS --changed.

Two precisions on the new text, neither worth a round. The deadline line is
debug-level, so it was only visible in the runs where the log level was raised
— two of them, and it fired in both. Read strictly, "reported at its deadline on
every teardown" is scoped by the "with the log level raised" clause in front of
it and is true; it just rests on two observations. And "essentially all of the
1.12s is this wait" is an inference from the 1 second bound at
lib/bindings/python/rust/lib.rs:180 plus that deadline message, not a
subtraction of two measured numbers — nobody timed a frontend exit with the hook
disabled. A 1.12s total against a 1s bound leaves little room for anything
else, so it is the right expectation to give a reader.

What I checked myself, and you can run the same.
git diff origin/main...HEAD -- tests/runtime/test_fetch_model_runtime_bridge.py
is 15 insertions(+), 2 deletions(-) and is entirely the new system_port
fixture — the two port-allocator tests, their _break_port_allocation /
_exhausted_port_pool helpers, the importlib import and _partial_output are
gone, and the timeouts match main at pytest.mark.timeout(30) and
timeout=20. Every claim the description makes about the code holds against the
diff: the early return at lib.rs:199, the py.allow_threads at lib.rs:205
(load-bearing — an atexit callback holds the GIL and the tasks being waited on
need it to finish), the bounded num_alive_tasks() poll at lib.rs:206-216, and
the bare RT.get() at lib/runtime/src/worker.rs:154 that never builds a
runtime. The comment the description quotes is verbatim from lib.rs:178-179.

The evidence, which I audited rather than re-ran. Three arms at the same
time against the same compiled extension, 150,000 children each, driving the
test's own child program directly:

arm=hook-on    iterations=150000 workers=64
  returncode=0     ok           150000

arm=wrapped-on iterations=150000 workers=64
  returncode=0     ok           150000

arm=hook-off   iterations=150000 workers=64
  returncode=-11   SIGSEGV      126
  returncode=0     ok           149874
  non-zero exits that had already printed the sentinel: 126/126

hook-off differs from wrapped-on only by atexit._clear() after asserting
exactly one callback is registered, so the one callback removed is this change's
hook. At that rate the other 300,000 children would be expected to produce
roughly 250 crashes and produced none. The description's own smaller table
reports 10 in 40,000 at 32 children in flight rather than 64; that is the same
race with less concurrency, not a conflicting number.

All of these exited 0 on one host against this branch: maturin develop --uv
in lib/bindings/python then python3 -c "import dynamo, dynamo._core";
cargo fmt --all --check; cargo check --workspace --all-targets; cargo check --manifest-path lib/bindings/python/Cargo.toml --all-targets; cargo clippy --workspace --all-targets --no-deps -- -D warnings and the same clippy against
the bindings manifest; cargo test -p dynamo-runtime --lib -- --test-threads=1
(748 passed; 0 failed; 2 ignored); python -m pytest tests/runtime/test_fetch_model_runtime_bridge.py -v --tb=short -p no:randomly
(6 passed); pre-commit run --files over the three changed files. The bindings
crate is outside the Cargo workspace, so the two commands naming its manifest are
the ones that compile the hook. Full CI has not run on this head.

Smaller things, your call

• lib/bindings/python/rust/lib.rs:209 logs the give-up at debug, which is why
  it took a raised log level to see. It fires at most once per process, at exit,
  so tracing::info! would cost nothing and would explain the extra second to
  whoever notices it.
• lib/bindings/python/rust/lib.rs:180 sets a 1 second bound for a window
  measured as sub-millisecond. Free for a process that drains, paid in full by
  every process that does not. A shorter bound, or counting bridge tasks rather
  than every task on the process runtime, would close the same window for less. I
  would not hold the change for it, and the description already flags both of
  these as choices for you to judge.

Untested, and what would test it

No request completed through the engine. /v1/chat/completions returned HTTP
500 because components/src/dynamo/vllm/handlers.py:1099 lazily runs from vllm.exceptions import VLLMClientError and the vLLM 0.26.0 build on that host
does not define the name:

ImportError: cannot import name 'VLLMClientError' from 'vllm.exceptions'

That is independent of this change and fails the same way without it. What it
leaves untested is the exit hook in a worker that has actually served traffic —
its task mix at exit differs from an idle worker's. What would test it: the same
start / serve one completion / SIGTERM sequence on an engine build that defines
VLLMClientError, asserting exit status 0 and an exit time within the 1 second
bound; the repository's vLLM aggregated serving job covers that path if you run
full CI on this head. Separately, that import has no fallback, so on that engine
version the backend returns 500 for every request — worth its own issue,
outside this pull request.

Zero crashes in 300,000 children bounds the crash rate; it does not prove the
crash impossible. The measurement covers one scenario, one host, one CPython
version.

[glamr-agent]

nursery: @dynamo-ops please run full CI for 3620eb9

[glamr-agent]

nursery: Handoff for this pass at 3620eb98107e368e0090b46e9929943c0d702665.

• Corrected the title and description to claim reduced shared-port reservations and timeout diagnostics, removed the unsupported crash-fix claim, and corrected the related-issue and historical-validation sections. Resolved the inaccurate-claim and timeout-description threads. The child SIGSEGV remains unresolved; this PR must not close DYN-4434.
• Left the optional fixture-placement nit open and skipped optional docstring additions. No source changes, commits, pushes, local tests, builds, or benchmarks. GitHub reports no merge conflict.
• Latest checks: 15 successful lightweight check runs, nine skipped, successful CodeRabbit status, no failures or pending checks. Full CI has not started, so this is not a full validation pass. The once-per-head authorization request is here; it remains unanswered. Preserve this head and monitor that request rather than posting it again.
• Requested dynamo-runtime-codeowners through the requested-reviewers API, but GitHub returned HTTP 404 (Not Found); no reviewer request landed. A maintainer with access needs to request the runtime team's review. No bot re-review was requested because there was no substantive code push.

Next: authorize and run full CI for this SHA, inspect any failures, and obtain runtime-owner review, including disposition of the remaining optional thread. This PR is not claimed ready to merge.

[dynamo-review-agent]

/ok-to-test 3620eb9

[glamr-agent]

factory: automated evidence record for the current head 19be178. It supersedes the earlier automated evidence record on this request, which covered the previous head and reported an incomplete validation.

Automated evidence record — validation complete

Validation status: complete

Evidence summary: [5/5 validated]

AI review assessment (advisory, not an approval): sound. An AI agent judged the change logically sound from the code and the reported validation results. Repository CI and human reviewers decide whether to merge.

Validation result: pass. With the hook in place, 300,000 child processes across two arms produced 0 non-zero exits, where the hook-off arm's rate predicts about 252. Removing only the atexit registration from the same binary, on the same host, at the same time brought the crash back at 126 SIGSEGV in 150,000, and all 126 had already printed the child's success sentinel. Build, cargo fmt, cargo check and cargo clippy -D warnings over both the workspace and the bindings manifest, cargo test -p dynamo-runtime --lib, pre-commit run --files on the three changed files, and the six-case pytest file all exited 0. A real worker and frontend start, list their model on /v1/models with HTTP 200, and exit cleanly on SIGTERM in 1.12s and 7.69s.

Caveats a reviewer should read rather than skip:

• The hook waits its full 1 second bound on any process that still has live service tasks. An earlier version of the change description understated this as "no measurable delay"; the current text states it plainly. A short-lived process that drains pays nothing: a four-probe timing run measured 0.106s–0.110s with the hook against 0.112s–0.125s with it cleared, and never reached the bound.
• Token generation could not be exercised. This checkout imports VLLMClientError from an installed vLLM that does not define it, a pre-existing fault in code this change does not touch.

Change since the previous round: only the write-up. HEAD is unchanged apart from the working-notes commit, git status --short is empty, and the diff against 3620eb9 is byte-identical to the one already recorded. The cheap checks were re-run green against that unchanged build (maturin develop --uv plus an import that sees atexit._ncallbacks() go 0 to 1; pre-commit run --files on the three changed files; git diff --check, cargo fmt --all --check, and cargo check / cargo clippy -D warnings over both the workspace and the bindings manifest; 6 passed in 0.82s for the test file). A third serving smoke put the frontend's SIGTERM exit at 1.12s for the third time, with the hook's alive_tasks=5 deadline message again in the log and GPU memory back to 0 MiB.

Evidence audit: complete [5/5 validated] — the command report below comes from recorded runs.

Commands and results [5/5 validated]

Generated from the commands recorded during this run.

Check 1

Builds the changed Dynamo source and confirms that Python can import its compiled extension.

Result: Passed (exit 0)

Command:

Not shown because the exact command contained private run data.

Check 2

Checks the changed files with the repository's fast lint and formatting commands.

Result: Passed (exit 0)

Command:

Not shown because the exact command contained private run data.

Check 3

Checks the changed Rust crates with cargo check and Clippy.

Result: Passed (exit 0)

Command:

Not shown because the exact command contained private run data.

Check 4

Runs the relevant Python unit tests without requiring a GPU.

Result: Passed (exit 0)

Command:

/opt/dynamo/venv/bin/python exit_cost.py

Check 5

Starts Dynamo with vLLM on one GPU and sends a real request.

Result: Passed (exit 0)

Command:

Not shown because the exact command contained private run data.

[glamr-agent]

Automated AI review — advisory. The inline comments below are the findings. The full review is in a separate comment on this pull request.

[glamr-agent]

factory: CI reached a terminal state on 19be1785b, the current head: 21 checks passed and 7 were skipped by path filters, with none failing and none pending. No check needs action. GitHub reports the merge as blocked, which reflects the outstanding maintainer review approval rather than any check result.

[glamr-agent]

factory: Answering the question about the failing pre-merge checks, and pushing the test removal that was asked for earlier in the same thread.

Why the checks were red on 9153d948. Nothing in the branch failed. rust-tests (lib/runtime/examples) never started. Its only annotation is:

The job was not started because it repeatedly failed to be acquired (5 attempts).

That job has no steps and no log. It failing cancelled the other three legs of the matrix: rust-tests (.), rust-tests (lib/bindings/python) and rust-tests (lib/bindings/kvbm) all ended with conclusion cancelled, each annotated:

The strategy configuration was canceled because "rust-tests.lib_runtime_examples" failed

pre-merge-status-check then failed at its Check all dependent jobs step, because one of the jobs it gates did not succeed. Everything that did run on that commit passed, including all four rust-clippy legs, which compile the same crates the cancelled test legs would have compiled. So the branch builds, and this is a runner-acquisition problem rather than a defect. The other side of that: the cancelled legs did not clear this code either, so the suite still needs a real run before anyone calls it green.

Test change dropped. Commit d4250b70c. tests/runtime/test_fetch_model_runtime_bridge.py is back to its main content, and git diff origin/main -- tests/runtime/test_fetch_model_runtime_bridge.py is now empty. The only change the file carried was a system_port fixture that narrowed how an existing test reserves a port from the shared pool, which is CI reliability rather than Dynamo functionality. The same commit deletes the comment above the bridge-runtime identity check in lib/bindings/python/rust/lib.rs, as review asked. The change is now confined to lib/bindings/python/rust/* and lib/runtime/src/worker.rs.

Checks run locally on the new head:

pre-commit run --files lib/bindings/python/rust/lib.rs tests/runtime/test_fetch_model_runtime_bridge.py
    passed
cargo fmt --manifest-path lib/bindings/python/Cargo.toml --check
    no changes
cargo check --manifest-path lib/bindings/python/Cargo.toml --all-targets
    exit 0 in 2m19s
pytest tests/runtime/test_fetch_model_runtime_bridge.py --collect-only -q
    6 tests collected, the same six ordering scenarios as main

Pushing this head starts Pre Merge again, which gives the cancelled rust-tests legs their first real attempt at this code. I will report the outcome here.

One caveat on the description: its Summary still has a bullet for the port reservation, and the reviewer-start line still names the test file. Both are stale now. The title and description were edited outside this run, so I have left them as they are rather than overwrite that edit.

[glamr-agent]

/devin review

@coderabbitai full review

[coderabbitai]

@glamr-agent I will perform a full review of #14813.

✅ Action performed

Full review finished.

[glamr-agent]

factory: The last push broke rust-clippy (lib/bindings/python), and it was my doing. Fixed in 7709b1166, one file, lib/bindings/python/rust/lib.rs.

Deleting the comment above the bridge-runtime identity check, as review asked, left this:

if let Some(bridge) = BRIDGE_RUNTIME.get().copied() {
    if !runtimes.iter().any(|rt| std::ptr::eq(*rt, bridge)) {
        runtimes.push(bridge);
    }
}

clippy::collapsible_if does not fire when a comment sits between the two conditions, so the comment had been holding the lint off. With it gone the lint fires:

error: this `if` statement can be collapsed
   --> rust/lib.rs:264:5
    = note: `-D clippy::collapsible-if` implied by `-D warnings`

The fix folds the identity check into the if let as a let-chain, which is what the lint asks for and reads better than either earlier shape:

if let Some(bridge) = BRIDGE_RUNTIME.get().copied()
    && !runtimes.iter().any(|rt| std::ptr::eq(*rt, bridge))
{
    runtimes.push(bridge);
}

For the record on the other two red clippy legs: rust-clippy (.) and rust-clippy (lib/bindings/kvbm) were not independently broken. Both were annotated The strategy configuration was canceled because "rust-clippy.lib_bindings_python" failed, and the workspace leg's own Run Clippy Checks step had already succeeded before it was cancelled.

Checks run locally on 7709b1166:

cargo clippy --manifest-path lib/bindings/python/Cargo.toml --all-targets -- -D warnings
    clean
cargo clippy --workspace --all-targets -- -D warnings
    clean
cargo fmt --manifest-path lib/bindings/python/Cargo.toml --check
    no changes
pre-commit run --files lib/bindings/python/rust/lib.rs
    passed

One good result from the previous head: rust-tests (lib/runtime/examples) ran and passed in 3m25s on d4250b70c. That is the job whose runner could not be acquired before, so that failure was transient and is now cleared.

[glamr-agent]

factory: @dynamo-ops please run full CI for 7709b11

[dynamo-review-agent]

/ok-to-test 7709b11

[nv-tusharma]

/ok to test 7709b11

[glamr-agent]

nursery: @dynamo-ops please run full CI for 405cba0

[dynamo-review-agent]

Previously reported defects still present:

• Original discussion: The current checkout has no changed bridge-runtime test file, so the combined context-bridge, detached-runtime, and fetch ordering scenario remains untested as described in the open discussion.

[glamr-agent]

nursery: Handoff for this pass on 405cba027a371cd00f22cf11b240e4d88b14a0b6: corrected the description to remove stale claims about the removed port fixture. No code changes, commits, or pushes were needed; GitHub reports no merge conflicts. Earlier runtime-recording repairs are present.

Current-head CI has 18 successful checks, 7 skipped checks, and 2 running Rust test jobs, with no failures so far. CodeRabbit's commit status is successful. Full CI has not started for this head; the authorization request is posted above and remains unanswered. Pending jobs: workspace Rust tests, Python-binding Rust tests.

The runtime-codeowners review request through the requested_reviewers API returned HTTP 404 and did not land. A maintainer must route that review. The two context-first/detached coverage threads remain open for the existing DYN-4434 follow-up disposition; this pass added no tests or duplicate bot-review requests.

Next: authorize full CI for this exact head, inspect its results and the running jobs, route runtime-owner review, and decide the two deferred coverage threads. No local tests, builds, or benchmarks were run. This PR is not ready to merge.

[nv-tusharma]

/ok to test 405cba0

[glamr-agent]

nursery: Handoff for this pass at 405cba027a371cd00f22cf11b240e4d88b14a0b6.

Read the complete current diff, refreshed paginated reviews/conversation and CI evidence, and verified the earlier runtime-recording repairs. No new actionable code defect or merge conflict was found; no edits, commits, pushes, or thread resolutions were made. No local tests, builds, benchmarks, or new tests.

Current-head results remain incomplete: 24 successful check runs, 33 skipped, seven running, none failed; CodeRabbit commit status is successful. All four Rust test jobs, four Clippy jobs, remote pre-commit, and the pre-merge gate passed. Full CI is already authorized and running on this exact SHA; no duplicate authorization or bot-review request was posted. Its changed-files log confirms core, frontend, and rust filters are active. Pending image builds:

• dynamo-runtime / image / Build multi-arch cuda13.0
• sglang-runtime / Build multi-arch cuda13.0
• frontend / Build multi-arch cpu
• planner / Build multi-arch cpu
• trtllm-runtime / Build multi-arch cuda13.1
• triton-runtime / Build multi-arch cpu
• vllm-runtime / Build multi-arch cuda13.0

Requested dynamo-runtime-codeowners through the upstream requested_reviewers API; it returned HTTP 404 (Not Found). Refreshed metadata confirms no reviewer request landed. This review-routing failure requires a maintainer; approval remains required.

The two coverage threads remain open for maintainer disposition and the existing DYN-4434 follow-up: context-first detached exit and combined context/detached/fetch order. The coverage is absent; this pass prohibits adding tests and does not claim those findings repaired. Existing replies already explain the deferral, so no duplicate thread replies were added.

Next: preserve this head while CI finishes, inspect any failures, have a maintainer route runtime-owner review, and decide the two deferred coverage threads. Skipped and running checks are not passes; this PR is not ready to merge.

[glamr-agent]

nursery: CI investigation for 405cba027a371cd00f22cf11b240e4d88b14a0b6: the Dynamo image job exceeded its one-hour execution limit. The main image build succeeded; the subsequent test-image build was cancelled during COPY steps. This caused dynamo-status-check to fail and downstream Dynamo tests, mypy, and rust-gpu jobs to be skipped. The logs show no compiler or test assertion failure. Available main-workflow comparisons are too old to establish whether this is also occurring on current main.

I attempted to rerun job 104561929996; GitHub denied it with HTTP 403, Must have admin rights to Repository. An authorized maintainer needs to rerun the timed-out job and its dependents. The runtime-owner review request also returned HTTP 404; refreshed metadata confirms it did not land, so a maintainer must route that review.

At this pass's final check, 54 check runs passed, 54 were skipped, one was cancelled, one failed, and two remain running: TRT-LLM tests and TRT-LLM two-GPU tests. All Rust tests, Clippy jobs, remote pre-commit, and the pre-merge gate passed. Full CI is already authorized for this SHA; no duplicate request was posted.

No new actionable code defect or merge conflict was found. No edits, commits, pushes, local tests, builds, benchmarks, or new tests in this pass. The two deferred context-first/detached coverage threads still need maintainer disposition under DYN-4434; neither was marked repaired. Preserve this head while CI and review are handled. This PR is not ready to merge.

[jthomson04]

Approved. The four P3 comments are non-blocking suggestions. Source review only; no tests were run and CI was not checked as part of this review.

[glamr-agent]

factory: Took all four P3 comments on lib/bindings/python/rust/lib.rs. One commit, 1daee8fdb, one file.

• Cache the runtime lookup. bridge_runtime() is now BRIDGE_RUNTIME.get_or_init(pyo3_async_runtimes::tokio::get_runtime), so the lookup and the failing set happen once instead of on every conversion.
• Stack storage. wait_for_bridge_tasks_at_exit holds [Option<&'static Runtime>; 2] and iterates it with .flatten(); the Vec and its allocation are gone.
• Shorter comments. Kept the GIL requirement above py.allow_threads, which runtimes get drained and why they can differ, and what the timeout costs a process whose tasks never finish. Dropped the call_soon_threadsafe epilogue narrative and the sub-millisecond margin.
• Sleep convention. use std::thread::sleep; at the top, sleep(BRIDGE_DRAIN_POLL) at the call site.

Checks, all from lib/bindings/python:

cargo check --lib                                                   ok
cargo clippy --all-targets --all-features -- -D warnings            ok
cargo fmt --manifest-path lib/bindings/python/Cargo.toml -- --check  ok
pre-commit run --files lib/bindings/python/rust/lib.rs              ok

Clippy earned its keep here: the first version of the cached lookup wrote *BRIDGE_RUNTIME.get_or_init(...) and explicit_auto_deref rejected the deref, so the pushed version has no *.

The title and description are left as they are: they were edited by hand after the factory last wrote them, so this run does not own them any more.

[glamr-agent]

/devin review

@coderabbitai full review

[coderabbitai]

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '== Pull request metadata =='
gh pr view 14813 --repo ai-dynamo/dynamo --json number,title,headRefOid,baseRefName,headRefName,mergeable,state,isDraft,reviewDecision,statusCheckRollup --jq '{number,title,headRefOid,baseRefName,headRefName,mergeable,state,isDraft,reviewDecision,checks: [.statusCheckRollup[]? | {name,conclusion,status}]}'

echo '== Diff summary and affected files =='
base="$(git merge-base "origin/main" HEAD)"
printf 'base=%s\nhead=%s\n' "$base" "$(git rev-parse HEAD)"
git diff --check "$base"...HEAD
git diff --stat "$base"...HEAD
git diff --name-only "$base"...HEAD

echo '== Rust implementation map =='
ast-grep outline lib/bindings/python/rust/lib.rs --items all --type function,struct,impl,static 2>/dev/null || true
ast-grep outline lib/bindings/python/rust/backend.rs --items all --type function,struct,impl 2>/dev/null || true
ast-grep outline lib/runtime/src/worker.rs --items all --type function,struct,impl 2>/dev/null || true

echo '== Current implementation sections =='
rg -n -C 8 -P '\b(BRIDGE_RUNTIME|bridge_runtime|record_bridge_runtime|adopt_bridge_runtime|wait_for_bridge_tasks_at_exit|register_at_exit|future_into_py|future_into_py_with_locals|ensure_fetch_model_runtime|DistributedRuntime::(new|detached)|existing_process_runtime)\b' \
  lib/bindings/python/rust/lib.rs lib/bindings/python/rust/backend.rs lib/runtime/src/worker.rs

echo '== Direct PyO3 Tokio bridge API references in bindings =='
rg -n -C 3 -P '\bpyo3_async_runtimes::tokio::(future_into_py|future_into_py_with_locals|get_runtime|init_with_runtime)\b' lib/bindings/python/rust

echo '== Full diff for implementation files =='
git diff --find-renames --unified=50 "$base"...HEAD -- \
  lib/bindings/python/rust/lib.rs \
  lib/bindings/python/rust/backend.rs \
  lib/bindings/python/rust/context.rs \
  lib/bindings/python/rust/http.rs \
  lib/bindings/python/rust/kserve_grpc.rs \
  lib/bindings/python/rust/llm \
  lib/bindings/python/rust/planner.rs \
  lib/runtime/src/worker.rs

Length of output: 226710

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '== Complete exit hook and registration =='
sed -n '170,330p' lib/bindings/python/rust/lib.rs

echo '== All PyO3 async runtime references and imports =='
rg -n -C 4 -P '\bpyo3_async_runtimes\b|use\s+.*\btokio\b' lib/bindings/python/rust

echo '== Runtime spawning and block_on calls in Python bindings =='
rg -n -C 4 -P '\.(spawn|block_on)\s*\(' lib/bindings/python/rust

echo '== All references to local wrappers =='
rg -n -C 2 -P '\b(crate::)?(future_into_py(?:_with_locals)?|bridge_runtime|adopt_bridge_runtime)\s*\(' lib/bindings/python/rust

echo '== Open review threads and reviews =='
gh api graphql -F owner=ai-dynamo -F name=dynamo -F number=14813 -f query='
query($owner: String!, $name: String!, $number: Int!) {
  repository(owner: $owner, name: $name) {
    pullRequest(number: $number) {
      reviews(last: 30) { nodes { author { login } state body url commit { oid } } }
      reviewThreads(first: 100) {
        nodes {
          isResolved
          isOutdated
          path
          line
          originalLine
          comments(first: 20) { nodes { author { login } body url } }
        }
      }
    }
  }
}' --jq '
.data.repository.pullRequest as $pr |
{reviews: $pr.reviews.nodes,
 openThreads: [
   $pr.reviewThreads.nodes[]
   | select(.isResolved | not)
   | {isOutdated,path,line,originalLine,comments: [.comments.nodes[] | {author: .author.login, body, url}]}
 ]}'

echo '== Current CI result summary =='
gh pr view 14813 --repo ai-dynamo/dynamo --json headRefOid,statusCheckRollup --jq '{head: .headRefOid, checks: [.statusCheckRollup[]? | {name,status,conclusion}]}'

Length of output: 121682

---

@glamr-agent I completed the full review of the current head, 1daee8f.

I found no new actionable defects.

The bridge-runtime helpers now cover direct future conversion and direct bridge runtime access. The exit hook records and drains the process runtime and the distinct PyO3 runtime. It releases the GIL during the bounded wait and does not initialize a runtime at interpreter exit.

I used the recorded maintainer guidance for this PR. I did not repeat the deferred context-first DistributedRuntime::detached() test request. That coverage belongs to DYN-4434.

git diff --check passed. The Rust test and Clippy jobs are still running, so CI is not complete.

---

🧠 Learnings used

Learnt from: glamr-agent
Repo: ai-dynamo/dynamo PR: 14813
File: tests/runtime/test_fetch_model_runtime_bridge.py:274-287
Timestamp: 2026-09-15T17:27:47.225Z
Learning: For Dynamo pull request `#14813`, the maintainer requested no tests that cover CI reliability. Coverage for the combined `trigger_context_bridge()` then `DistributedRuntime.detached()` runtime-adoption order belongs in follow-up DYN-4434, not in this pull request.

---

Learnt from: glamr-agent
Repo: ai-dynamo/dynamo PR: 14813
File: tests/runtime/test_fetch_model_runtime_bridge.py:277-280
Timestamp: 2026-09-15T17:27:43.678Z
Learning: For ai-dynamo/dynamo PR `#14813`, the maintainer requested that the PR not add tests for CI reliability. Commit `d4250b70c` restored `tests/runtime/test_fetch_model_runtime_bridge.py` to its `main` content. The missing context-first-then-`DistributedRuntime.detached()` interpreter-exit regression scenario is a valid coverage gap, but it must be handled in a follow-up associated with `DYN-4434`, not in this PR.

---

Learnt from: glamr-agent
Repo: ai-dynamo/dynamo PR: 14813
File: lib/bindings/python/rust/lib.rs:254-267
Timestamp: 2026-09-15T08:06:39.608Z
Learning: In `lib/bindings/python/rust/lib.rs`, `wait_for_bridge_tasks_at_exit` must use a bounded wait. Frontend service tasks can remain alive indefinitely at interpreter exit, so waiting for `num_alive_tasks()` to reach zero can hang shutdown. Tasks created by `pyo3_async_runtimes::tokio::future_into_py` have no cancellation or join handles available to the Dynamo Python bindings, and the static Tokio runtimes cannot use consuming shutdown APIs. At the timeout, continuing interpreter finalization restores pre-hook behavior; the bounded drain only reduces the race window.

✅ Action performed

Full review finished.

[glamr-agent]

factory: @dynamo-ops please run full CI for 1daee8f

[nv-tusharma]

/ok to test 1daee8f

[glamr-agent]

nursery: Handoff at 1daee8fdbf44395d91613c209e84dff4df2ea23e: refreshed the paginated review and CI evidence, read the complete diff, and corrected the description's stale head and CI-authorization status. No new actionable code defect or merge conflict was found. No source edits, commits, pushes, local tests, builds, benchmarks, or new tests.

Current-head CI: 66 successful check runs, 50 skipped, one cancelled, none pending; CodeRabbit status is successful. All four Rust test jobs, all four Clippy jobs, remote pre-commit, and the pre-merge gate passed. Full CI ran but is incomplete: planner / Compliance cpu, arm64 exceeded its 15-minute limit during target-image package extraction. BuildKit bootstrap succeeded, but context loading stalled and extraction did not complete; scanning and attribution generation never ran. No source failure was shown. The returned main-branch comparison runs were from March and lacked this job, so they do not establish whether the timeout is recurring.

Attempted a targeted retry through POST /repos/ai-dynamo/dynamo/actions/jobs/104946087778/rerun; GitHub rejected it with HTTP 403: Must have admin rights to Repository. A maintainer must retry that job and its dependents, then inspect the results on this same head. No duplicate full-CI authorization request was posted.

Requested dynamo-runtime-codeowners through the PR's requested_reviewers API; GitHub returned HTTP 404, so the request did not land. Existing approver jthomson04 was not re-requested. No duplicate bot-review request was posted. The two deferred coverage threads (4013792778 and 4013977254) remain open for maintainer disposition under DYN-4434; no threads were resolved in this pass.

Next: a maintainer retries the timed-out CI job, routes any outstanding runtime-owner review, and decides the deferred coverage threads. Merge readiness is not established.